From 2dbeb36c88da30fe62b241f610321a36f9428de8 Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Mon, 24 Aug 2026 09:05:03 +0100 Subject: [PATCH 1/2] feat(model,sdk/go): core.try, and the binding item and the failure share MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A container binds a name for the children it owns by declaring it as an ordinary output of itself, read as `{{steps..}}`. One mechanism for both of ADR-0013's bindings: a loop's `item` and a try's `error`. It costs no namespace root and no bare token, which is why it is the answer. ADR-0014 closed the roots on the argument that every structural idea Hatua adds is a name taken away from users, and a binding owned by a container is exactly such an idea. A step id already sits one segment below `steps.`, so nesting needs no shadowing rule: two containers are two step ids. `t: item` was documented, reachable and resolved by nothing, and because the checker treats `item` as matching everything the gap surfaced as a type check that always passed rather than as an error. It now resolves by reading the loop's `list` as a Reference, typing it against the loop step's own scope, and taking the element shape of the list it names. Where it cannot resolve it stays `item` and stays permissive, and LOOP_LIST_NOT_A_LIST reports the list instead — necessary because `list` is a `ref` field, which FIELD_KIND_TYPES maps to `unknown`, so the ordinary Slot check accepts anything written there. core.try nests twice: a body under `steps:` and a fallback under `handler:`. A key rather than two branches under reserved labels, because a branch's identity is its label — free text a user renames — and a region renameable out of existence is not a region. The two regions are siblings, so the body cannot see the handler and the handler cannot read the body's steps, with no code saying so; which of them completed before the failure is not a fact the document holds. The retry policy sits in `with:` as ordinary manifest fields. `until` left `with:` because FIELD_KIND_TYPES has no mappable boolean; an attempt count is a number and `number` IS mappable, so that argument is absent here and following it anyway would copy a conclusion without its reason. A try discharges a block's return obligation only when both regions return. Its body always runs, but a failure part-way through it enters the handler instead, so the guarantee is a conjunction rather than the repeat's "the body always runs". Also fixed, found by sweeping the walkers rather than by a failing test: TypeScript's `callsOf` skipped the handler, so recursion through one went unreported while Go caught it. Both languages now walk it, pinned by a scenario. The catalogue's `core.for_each` declared `items`/`t: object`, which the schema and ADR both call `list`/`t: item`; the playground seed was written against the old key and iterated a single object. Both corrected, and `seed.test.ts` now holds the seed to the catalogue it is actually served — the layer the rules corpus, which supplies its own manifests, cannot reach. --- CONTEXT.md | 37 +- docs/adr/0013-control-flow-nests.md | 127 ++++- docs/handoff.md | 6 + source/apps/playground/package.json | 6 +- source/apps/playground/src/seed.test.ts | 61 +++ source/apps/playground/src/workflow-store.ts | 9 +- .../invalid/handler-is-not-a-list.yaml | 18 + .../invalid/handler-step-missing-use.yaml | 18 + .../definition/rules/try-and-item.yaml | 501 ++++++++++++++++++ source/conformance/definition/valid/full.yaml | 34 +- source/conformance/manifest/catalogue.yaml | 61 ++- .../packages/document/src/round-trip.test.ts | 57 ++ source/packages/model/src/blocks.ts | 6 +- .../model/src/generated/diagnostics.ts | 18 + source/packages/model/src/loops.test.ts | 226 +++++++- source/packages/model/src/scope.ts | 184 ++++++- source/packages/model/src/slots.ts | 40 ++ source/packages/model/src/tree.ts | 11 +- source/packages/model/src/validity.ts | 111 +++- .../react/src/layouts/StepList.stories.tsx | 27 +- .../react/src/layouts/StepList.test.tsx | 61 +++ .../packages/react/src/layouts/StepList.tsx | 60 ++- .../schema/src/generated/component.ts | 3 +- .../schema/src/generated/definition.ts | 17 +- source/packages/services/src/steps.ts | 21 +- source/pnpm-lock.yaml | 12 + source/schemas/component-manifest.schema.yaml | 13 +- source/schemas/definition-diagnostics.yaml | 47 ++ .../schemas/workflow-definition.schema.yaml | 47 +- source/sdk/go/definition.go | 44 ++ source/sdk/go/diagnostics.gen.go | 6 + source/sdk/go/load.go | 5 + source/sdk/go/slots.go | 191 ++++++- source/sdk/go/slots_test.go | 208 ++++++++ source/sdk/go/validity.go | 101 +++- 35 files changed, 2322 insertions(+), 72 deletions(-) create mode 100644 source/apps/playground/src/seed.test.ts create mode 100644 source/conformance/definition/invalid/handler-is-not-a-list.yaml create mode 100644 source/conformance/definition/invalid/handler-step-missing-use.yaml create mode 100644 source/conformance/definition/rules/try-and-item.yaml diff --git a/CONTEXT.md b/CONTEXT.md index 3a2f31c..7bceb21 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -114,9 +114,9 @@ _Avoid_: state, global, parameter, field **Mapping**: A **Step** (`core.map`) whose outputs are the entries the user wrote into it rather than anything -its **Component Manifest** declares. It is the third verb Hatua interprets structurally, alongside -`core.fork` and `core.for_each` — and the only one read from a field's *value* rather than from its -position in the tree. Each entry is a key, a **Template** and a declared type, so a downstream +its **Component Manifest** declares. It is the one verb Hatua interprets structurally by reading a +field's *value* rather than a position in the tree — `core.fork`, `core.for_each`, `core.repeat` and +`core.try` are all read from where their children sit. Each entry is a key, a **Template** and a declared type, so a downstream **Step** addresses `{{steps.s8.headline}}` and type-checks against it like any other output. _Avoid_: transform, set variables, assign, formula step @@ -150,6 +150,37 @@ distinguishes it from `core.for_each` — a list may be empty — and what lets by `core.set_var`. Nothing in the document bounds the iterations — a runner imposes its own ceiling. _Avoid_: while, do-while, until-loop, retry +**Try**: +A container **Step** (`core.try`) with **two** child regions where every other container has one: a +protected body under `steps:`, and a fallback under `handler:` that runs if the body fails. +**Wrapping one Step is retry; wrapping a region is fallback**, so one verb serves both. The two +regions are *siblings* — the body cannot see the handler, and the handler cannot read the body's +**Steps**, because which of them completed before the failure is not a fact the document holds. Its +retry policy lives under `with:` as ordinary **Component Manifest** fields, because a count and a +delay are numbers and `number` is a mappable field kind — the argument that put a **Repeat**'s +`until` beside `steps:` was about booleans and does not reach here. It discharges a **Block**'s +obligation to reach a `core.return` only when *both* regions return. Error-type matching needs no +matcher: a **Fork** inside the handler branches on the failure's `type`. +_Avoid_: catch, rescue, error handler, on-error branch + +**Binding**: +A name a container puts into the scope of the children it owns — a **Loop**'s `item`, a **Try**'s +`error`. **A binding is an output of the container Step itself**, read as +`{{steps..}}`, which is one mechanism rather than two and costs no namespace root +and no bare token: ADR-0014 closed the roots precisely so a structural idea could not take a word +away from users, and a **Step** id already sits one segment below `steps.`. Nesting needs no +shadowing rule, because two containers are two Step ids. +_Avoid_: variable, loop variable, context, implicit + +**Item**: +A **Loop**'s **Binding**: one element of the list its `list` field names, read as +`{{steps..item}}`. Declared `t: item` in the **Component Manifest**, which is the one type +whose meaning depends on the **Step** declaring it — the shape is not in the manifest at all, but is +resolved by following `list` to its source output's `of:`. Where it cannot resolve, `item` stays +`item` and matches anything, and the wrongness is reported against the *list* rather than guessed +into a shape. +_Avoid_: element, current, each, loop var + **Derived Layout**: The rule that a **Step**'s position on the flow map is computed from the tree on every render and never persisted. This is what guarantees a hand-edited **Workflow Definition** and the map can never diff --git a/docs/adr/0013-control-flow-nests.md b/docs/adr/0013-control-flow-nests.md index 4027760..e94c3f9 100644 --- a/docs/adr/0013-control-flow-nests.md +++ b/docs/adr/0013-control-flow-nests.md @@ -89,7 +89,7 @@ that gives it a reader**, and amends this ADR when it does. enclosing Block's `outputs:`. - **`core.try`** — a region with a retry policy and a fallback handler. **Wrapping one Step is retry; wrapping a region is fallback**, so one verb serves both. Error-type matching needs no matcher of - its own: a `core.fork` inside the handler branches on the failure. + its own: a `core.fork` inside the handler branches on the failure. Its shape is below. **Invoking a Block is not a verb.** `core.call` was the first draft and it was refused once the verb namespace closed (ADR-0014): a call is `use: block.`, resolved against `blocks:` instead of @@ -103,7 +103,8 @@ not a run-time depth limit. `core.try` exposes the failure to its **handler** children and not to its body, the way `core.for_each` exposes `item` — a container putting a binding into the scope of children it owns, -which the Fork's per-branch scoping already establishes. +which the Fork's per-branch scoping already establishes. **Both bindings are the same mechanism, and +the section below says what it is** rather than leaving one defined by analogy to the other. The names are `core.*` because **Hatua ships them**, which is what that root means (ADR-0014) — `core.schedule`, `core.manual` and `core.end` are `core.*` too and none of them is control flow. @@ -196,7 +197,8 @@ condition fork is first-match-wins, so one whose every branch is conditional can and fall straight through. A return inside a `core.for_each` body exits the Block early and is perfectly legal, but it never discharges the obligation, because the list may be empty and the body may never run. A `core.repeat`'s body does discharge it, for the mirror-image reason, and the section below -settles why. That is the same reasoning that keeps sibling branches out of scope, applied to time +settles why. A `core.try` discharges it only when both its regions do, which is this same +all-branches reasoning asked of a region that may or may not execute. That is the same reasoning that keeps sibling branches out of scope, applied to time instead of to paths. Steps sitting after a return on the same path can never run, and are reported the way an unconditional Branch that swallows the ones behind it already is. @@ -239,10 +241,9 @@ different name. **A repeat binds nothing.** `core.for_each` exposes `item`, and it can: `item` is resolved by following the loop's `list` back to its source output, so its type is derivable from the document. A repeat has no list. An iteration index or count would therefore be a binding nothing declares and -nothing types — and it would have to live somewhere, which under ADR-0014's closed roots means a -seventh root or a second bare token beside `TRIGGER`. Both are a permanent cost for a counter -`core.set_var` already writes, which is the trade the section below makes once and should not make -twice. +nothing types — and a binding with no type is the one thing the mechanism below cannot carry, because +that mechanism is an ordinary manifest output. `core.set_var` already writes a counter, which is the +trade the section on loop state makes once and should not make twice. **Nothing bounds the iterations, and that is a decision rather than an omission.** Recursion is refused above because it is a property of the *document* — a cycle in the call graph, decidable by @@ -253,6 +254,118 @@ keep. **Bounding is the Host runner's obligation**: a runner imposes its own ite fails the execution when it is reached, the way it already owns timeouts and retries. Hatua does not execute, so that is the one place the contract can honestly sit. +## A container's binding is an output of the container + +`core.for_each` exposes `item` and `core.try` exposes the failure. That is one idea asked twice, and +it gets one mechanism: **a container binds a name for the children it owns by declaring it as an +ordinary output of itself**, read as `{{ steps.. }}`. + +```yaml +- id: each + use: core.for_each + with: { list: "{{ steps.fetch.messages }}" } + steps: + - { id: send, use: component.email.send, with: { to: "{{ steps.each.item.address }}" } } + +- id: guard + use: core.try + with: { attempts: 3, backoff_ms: 500 } + steps: + - { id: publish, use: component.s3.upload } + handler: + - { id: warn, use: component.chat.post, with: { text: "{{ steps.guard.error.message }}" } } +``` + +**This costs no namespace root and no bare token, which is the whole reason it is the answer.** +ADR-0014 closed the roots on the argument that "every structural idea Hatua adds is a name taken away +from users" — and a binding owned by a container is exactly such an idea. A bare `item` is the token +that argument refuses. A seventh root costs a word forever, for two bindings and every future one. A +Step id already sits one segment below `steps.`, so a binding hung off the container is a name inside +a name the user chose, and collides with nothing. + +**Nesting needs no rule, and that is the test the alternatives fail.** Two nested loops are two Step +ids, so `steps.outer.item` and `steps.inner.item` are different paths and neither shadows the other. +A bare `item` would need a shadowing rule — innermost wins — and then an escape hatch for reaching +the outer one, which is a second concept and a worse one, because the reader has to count enclosing +loops to know what a word means. Here they read the id and are done. + +**The failure's shape is declared where the verb is.** `core.try`'s manifest declares +`error: {message, type, step}` the way any component declares its outputs, so the type checker, the +completion list and the reference tree need no code at all for it — which is also what makes +"a `core.fork` inside the handler branches on the failure" true rather than aspirational: `error.type` +is an ordinary text member. Hatua ships the verb, so Hatua declares the shape, and every Host runner +fills it in. + +**`item` is the one output whose type is not in the manifest**, and `t: item` is what says so. It is +resolved by reading the loop's `list` field as a Reference, typing that path against the loop Step's +own scope, and taking the element shape of the list it names — the `of:` the source output declared. +This is the debt this decision pays off: `t: item` was documented, reachable and resolved by nothing, +and because the checker treats `item` as matching everything, the gap surfaced not as an error but as +a type check that always passed. + +Where it cannot resolve — `list` absent, not a plain Reference, naming nothing, or naming something +that is not a list — `item` stays `item` and stays permissive. Guessing `object` would be a shape +nothing declared, and every `{{ steps.each.item.}}` would then type-check against members the +manifest never had. **The wrongness is reported instead**: `LOOP_LIST_NOT_A_LIST` names a loop whose +`list` has a known type that is not a list. It has to be its own rule because `list` is a `ref` field +and `FIELD_KIND_TYPES` maps `ref` to `unknown`, so the ordinary Slot check accepts anything written +there — the same "a check that always passes" failure, one layer up. + +## `core.try` has two regions, and both of them are `steps:`-shaped + +**The body is `steps:` and the handler is `handler:`.** `steps:` is already "the children a container +owns", and a try's body is exactly that, so the traversal covers it unchanged. + +**Two `branches:` under reserved labels was the alternative and is refused.** A Branch's identity is +its `label`, which is free text the user renames — so the meaning of the document would depend on a +display name, and a region could be renamed out of existence. It also costs the schema its first +reserved word, which is the thing ADR-0014 spent a whole decision removing. A key cannot collide with +anything a user chooses, because **nothing inside a step is user-named**: `id`, `use`, `name`, +`with`, `branches`, `steps`, `until` and `handler` are a closed set the schema owns. + +The cost is that a region is now a third thing a traversal can forget, beside a Branch's steps and a +loop's body. That is paid where it is cheapest: `walkSteps` and `stepLists` are the only walks, in +each language, and a `handler:` fixture in `conformance/definition/invalid/` holds both loaders to +reaching it. + +**The retry policy is in `with:`, and the `until` precedent does not reach it.** `until` had to leave +`with:` because `FIELD_KIND_TYPES` has no mappable boolean at all — a condition there would have +type-checked as *text*, and half the contract would have been gone. An attempt count and a backoff +are **numbers**, and `number` is a mappable field kind, so that argument is simply absent here. +Putting them in a structural key by analogy would be copying a conclusion without its reason, and +would cost a schema key, a diagnostic and a form control that a manifest field gives for nothing. + +**A `core.try` discharges a Block's return obligation only when BOTH regions return.** The body always +runs, which on its own looks like the `core.repeat` argument — but a failure part-way through the body +is precisely what a try exists to admit, and that path leaves the body unfinished and enters the +handler instead. So every path out of a try goes through the body *or* through the handler, and a +region that may skip its return leaves one of them open. That is the Fork's all-branches reasoning +asked of two regions, one of which is conditional; it is not the repeat's "guaranteed to run at all". + +## What a handler's children can read + +**The failure, everything above the try, and nothing from the body.** + +The two regions are **siblings**, so the body cannot see the handler and the handler cannot see the +body's Steps — and this needs no code, because it is the rule that already keeps a Fork's branches out +of each other's scope. It is also the right rule for the right reason. The body failed *somewhere*; +which of its Steps completed before it did is not a property of the document, so offering them would +make scope an intersection over paths. That is the analysis this ADR refuses edges in order to avoid, +arriving through a different door. + +The try Step itself is in scope **only** inside its handler, which is the one place its binding means +anything: + +| reading from | sees `steps.` | +| --- | --- | +| the body | no — the body is what produces the failure | +| the handler | yes — it is the failure being handled | +| a Step after the try | no — whether there was a failure at all is a run-time fact | + +The last row is the one worth stating. A Step after the try is on a path where either the body +succeeded or the handler ran, and "the failure, or nothing" is a value whose existence depends on the +run. Offering it would be the same intersection, one level out. + ## Loop state is a Board variable A repeated region usually has to carry something backwards — the reviewer's feedback reaching the diff --git a/docs/handoff.md b/docs/handoff.md index 9e24c67..ac7e9a4 100644 --- a/docs/handoff.md +++ b/docs/handoff.md @@ -252,6 +252,12 @@ field under `with:`, so neither is reached through the **Component Manifest** an than as a field of their own. Whatever surface holds one holds the other; a builder that could author a repeat's condition and not a fork's would be a worse gap than having neither. +**A `core.try` adds no row, and that is the point of where its retry policy sits.** A container with +a structural key needs a surface of its own; a container whose configuration is ordinary `with:` +fields does not. An attempt count and a backoff are numbers, `number` is a mappable field kind, and +the argument that pushed a condition out of `with:` was about booleans — so a try's fields are edited +in the Step editor exactly as any Component's are, and the table stays four rows long. + ### The Template input `min-height` 40px (76px for textarea), `--radius-md`, 1px `--border-strong`, `--surface-card`, diff --git a/source/apps/playground/package.json b/source/apps/playground/package.json index 1c70799..8dfe6fe 100644 --- a/source/apps/playground/package.json +++ b/source/apps/playground/package.json @@ -19,9 +19,13 @@ "react-dom": "^19.2.0" }, "devDependencies": { + "@hatua/expressions": "workspace:*", + "@hatua/model": "workspace:*", + "@hatua/schema": "workspace:*", "@hatua/sdk": "workspace:*", "@types/react": "^19.2.2", "@types/react-dom": "^19.2.1", - "@vitejs/plugin-react-swc": "^4.3.3" + "@vitejs/plugin-react-swc": "^4.3.3", + "yaml": "^2.8.1" } } diff --git a/source/apps/playground/src/seed.test.ts b/source/apps/playground/src/seed.test.ts new file mode 100644 index 0000000..bd08bb5 --- /dev/null +++ b/source/apps/playground/src/seed.test.ts @@ -0,0 +1,61 @@ +/** biome-ignore-all lint/correctness/noNodejsModules: a Node test reading the catalogue from disk; the playground's own code never does. */ +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { coreFunctions, validate } from '@hatua/expressions' +import { indexManifests, scopeFor, validateDefinition } from '@hatua/model' +import type { Manifest, WorkflowDefinition } from '@hatua/schema' +import { describe, expect, it } from 'vitest' +import { parse } from 'yaml' +import { SEED } from './workflow-store' + +/** + * The seed workflow, held against the catalogue the playground actually serves. + * + * This is the layer the conformance corpus cannot reach. The corpus supplies its + * own manifests per scenario, so a rename in `conformance/manifest/catalogue.yaml` + * — a field key, an output's type — leaves every scenario green while the first + * screen a person sees fills with markers. That is exactly what happened to the + * loop below: it was written against a field key the catalogue no longer has. + * + * Two assertions rather than one. "Nothing is reported" catches the rename; + * "`item` resolves to the element the source declared" catches the quieter + * failure, where `t: item` goes unresolved, the checker treats it as matching + * everything, and a wrong path type-checks clean. + */ + +const CATALOGUE: Manifest[] = parse( + readFileSync( + fileURLToPath(new URL('../../../conformance/manifest/catalogue.yaml', import.meta.url)), + 'utf8', + ), +).components + +const seed = (): WorkflowDefinition => parse(SEED) + +describe('the seed workflow', () => { + /* + * The exact set rather than "nothing", because the seed is not meant to be + * clean: s1's connection is left empty on purpose, so the Flow tab has a + * marker to show on the first screen anyone sees. Pinning the set catches a + * diagnostic appearing AND the deliberate one going away, where a count or a + * "nothing new" check would miss one of the two. + */ + it('reports exactly the one problem it is seeded with, against the catalogue it serves', () => { + const found = validateDefinition(seed(), indexManifests(CATALOGUE)).all + expect(found.map((one) => `${one.code} on ${one.stepId ?? one.triggerId ?? ''}`)).toEqual([ + 'FIELD_REQUIRED on s1', + ]) + }) + + it('resolves its loop’s `item` to the element its list declares', () => { + const scope = scopeFor(seed(), { board: null, id: 's5' }, CATALOGUE) + const context = { scope, functions: coreFunctions() } + + expect(validate('{{ steps.s4.item.filename }}', 'text', context)).toEqual([]) + // The gate is on, rather than switched off by an unresolved `item`: a text + // member is refused where a number is declared, and a member nothing + // declares is not silently accepted. + expect(validate('{{ steps.s4.item.filename }}', 'number', context)).not.toEqual([]) + expect(validate('{{ steps.s4.item.bytes }}', 'number', context)).toEqual([]) + }) +}) diff --git a/source/apps/playground/src/workflow-store.ts b/source/apps/playground/src/workflow-store.ts index 146611d..838cbbb 100644 --- a/source/apps/playground/src/workflow-store.ts +++ b/source/apps/playground/src/workflow-store.ts @@ -93,16 +93,19 @@ steps: steps: [] - id: s4 use: core.for_each - name: "Each message" + name: "Each attachment" with: - items: "{{ triggers.overnight.message }}" + # A list, and the loop's binding is one element of it: \`{{steps.s4.item}}\` + # carries the members \`attachments\` declares, with no shape written here. + list: "{{ triggers.overnight.message.attachments }}" steps: - id: s5 use: component.email.send - name: "Send the digest" + name: "Forward it on" with: connection: mailbox to: me@example.com + subject: "{{ steps.s4.item.filename }}" ` interface Stored { diff --git a/source/conformance/definition/invalid/handler-is-not-a-list.yaml b/source/conformance/definition/invalid/handler-is-not-a-list.yaml new file mode 100644 index 0000000..dd62665 --- /dev/null +++ b/source/conformance/definition/invalid/handler-is-not-a-list.yaml @@ -0,0 +1,18 @@ +# expect: SCHEMA_INVALID +# +# `handler` holds a step list. A mapping there is the shape a hand-edit reaches +# for when it thinks of the handler as one step rather than a region — and a +# reader that accepted it would have a `core.try` whose fallback is a step nothing +# walks, which is the same silence an unwalked region gives. +id: wf +name: W +version: 1 +status: draft +steps: + - id: guard + use: core.try + steps: + - { id: s1, use: component.email.send } + handler: + id: s2 + use: component.email.send diff --git a/source/conformance/definition/invalid/handler-step-missing-use.yaml b/source/conformance/definition/invalid/handler-step-missing-use.yaml new file mode 100644 index 0000000..c1af79a --- /dev/null +++ b/source/conformance/definition/invalid/handler-step-missing-use.yaml @@ -0,0 +1,18 @@ +# expect: SCHEMA_INVALID +# +# A `core.try`'s handler is a step list like any other, and the traversal has to +# reach it. This fixture exists because a region a walk forgets is a region no +# rule ever sees: the step below is missing the one key every step needs, and a +# reader that only descended into `steps:` and `branches:` would load this +# document clean while the builder refused to open it. +id: wf +name: W +version: 1 +status: draft +steps: + - id: guard + use: core.try + steps: + - { id: s1, use: component.email.send } + handler: + - id: s2 diff --git a/source/conformance/definition/rules/try-and-item.yaml b/source/conformance/definition/rules/try-and-item.yaml new file mode 100644 index 0000000..7d4e69b --- /dev/null +++ b/source/conformance/definition/rules/try-and-item.yaml @@ -0,0 +1,501 @@ +about: >- + `core.try` and the `item` binding, as both languages must report them. + + These are one decision asked twice. A container that binds a name for the + children it owns binds it as an OUTPUT OF ITSELF — `{{steps..item}}` + and `{{steps..error}}` — so neither costs a namespace root nor a + bare token, which is what ADR-0014 closed the roots to prevent, and two nested + containers cannot shadow each other because two Steps cannot share an id on one + Board. + + A try's two regions are siblings. The handler sees the try itself and therefore + the failure; the body does not, because the body is what produces it; a Step + after the try does not either, because whether there was a failure at all is a + run-time fact. The handler cannot read the body's Steps, for the reason sibling + Branches cannot read each other's — which of them completed before the failure + is not a property of the document. + + The load-bearing pair for the return obligation is here: a try discharges it + only when BOTH regions do. The body always runs, but a failure part-way through + it leaves the body unfinished and enters the handler instead, so the guarantee + is a conjunction rather than the repeat's "the body always runs". + + LOOP_LIST_NOT_A_LIST is the one rule that makes `t: item` observable at all. + `list` is a `ref` field and `FIELD_KIND_TYPES` maps `ref` to `unknown`, so + without it a loop pointed at a number type-checks clean while `item` resolves to + nothing and matches everything downstream. + +manifests: + - kind: component + use: component.email.send + name: Send email + fields: + - { k: to, label: To, kind: text, req: true } + outputs: [] + - kind: component + use: component.inbox.fetch + name: Fetch inbox + fields: [] + outputs: + - k: messages + label: Messages + t: list + of: + - { k: subject, label: Subject, t: text } + - k: sender + label: Sender + t: object + of: + - { k: address, label: Address, t: text } + - { k: count, label: Count, t: number } + - kind: component + use: component.inbox.threads + name: Fetch threads + fields: [] + outputs: + - k: threads + label: Threads + t: list + of: + - k: entries + label: Entries + t: list + of: + - { k: body, label: Body, t: text } + - kind: component + use: core.fork + name: Branch + fields: [] + outputs: [] + - kind: component + use: core.for_each + name: For each + fields: + - { k: list, label: List, kind: ref, req: true } + outputs: + - { k: item, label: Item, t: item } + - kind: component + use: core.repeat + name: Repeat + fields: [] + outputs: [] + - kind: component + use: core.try + name: Try + fields: + - { k: attempts, label: Attempts, kind: number } + - { k: backoff_ms, label: Backoff, kind: number } + outputs: + - k: error + label: Error + t: object + of: + - { k: message, label: Message, t: text } + - { k: type, label: Type, t: text } + - { k: step, label: Step, t: text } + - kind: component + use: core.return + name: Return + fields: [] + outputs: [] + - kind: trigger + use: core.schedule + name: Schedule + fields: [] + outputs: [] + +scenarios: + # ---- core.try, and its two regions ---------------------------------------- + + - name: a try with a body and a handler reports nothing + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - id: guard + use: core.try + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + handler: + - { id: s2, use: component.email.send, with: { to: "ops@dane.dev" } } + expect: [] + + - name: a try with no handler has nowhere for a failure to go + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - id: guard + use: core.try + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + expect: + - { code: TRY_HAS_NO_HANDLER, blocks: publish, stepId: guard } + + - name: a try with no body protects nothing + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - id: guard + use: core.try + handler: + - { id: s2, use: component.email.send, with: { to: "ops@dane.dev" } } + expect: + - { code: TRY_HAS_NO_BODY, blocks: publish, stepId: guard } + + - name: an empty try is both halves missing, and says so twice + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - { id: guard, use: core.try } + expect: + - { code: TRY_HAS_NO_BODY, blocks: publish, stepId: guard } + - { code: TRY_HAS_NO_HANDLER, blocks: publish, stepId: guard } + + - name: an empty body is not the loop code, because a try is not a loop + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - { id: each, use: core.for_each } + - { id: guard, use: core.try, handler: [{ id: s2, use: component.email.send, with: { to: "a@b.c" } }] } + expect: + - { code: FIELD_REQUIRED, blocks: publish, stepId: each, fieldKey: list } + - { code: LOOP_HAS_NO_BODY, blocks: publish, stepId: each } + - { code: TRY_HAS_NO_BODY, blocks: publish, stepId: guard } + + - name: a handler's steps are walked, so a required field left empty inside one is reported + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - id: guard + use: core.try + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + handler: + - { id: s2, use: component.email.send, with: {} } + expect: + - { code: FIELD_REQUIRED, blocks: publish, stepId: s2, fieldKey: to } + + - name: a step id repeated between a try's two regions is still two steps on one board + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - id: guard + use: core.try + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + handler: + - { id: s1, use: component.email.send, with: { to: "ops@dane.dev" } } + expect: + - { code: STEP_ID_DUPLICATE, blocks: publish, stepId: s1 } + + - name: a try's retry policy is an ordinary manifest field, so an unfilled optional one is silent + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - id: guard + use: core.try + with: { attempts: 3 } + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + handler: + - { id: s2, use: component.email.send, with: { to: "ops@dane.dev" } } + expect: [] + + # ---- a try and the return obligation -------------------------------------- + + - name: a try discharges the obligation when both regions return + definition: + id: wf + name: W + version: 1 + status: draft + steps: [] + blocks: + - id: ask + outputs: [{ k: answer, label: Answer, t: text }] + steps: + - id: guard + use: core.try + steps: + - { id: ret, use: core.return, with: { answer: "yes" } } + handler: + - { id: fallback, use: core.return, with: { answer: "no" } } + expect: [] + + - name: a body that returns and a handler that does not leaves the failure path open + definition: + id: wf + name: W + version: 1 + status: draft + steps: [] + blocks: + - id: ask + outputs: [{ k: answer, label: Answer, t: text }] + steps: + - id: guard + use: core.try + steps: + - { id: ret, use: core.return, with: { answer: "yes" } } + handler: + - { id: warn, use: component.email.send, with: { to: "ops@dane.dev" } } + expect: + - { code: BLOCK_PATH_WITHOUT_RETURN, blocks: publish, blockId: ask } + + - name: a handler that returns and a body that does not leaves the success path open + definition: + id: wf + name: W + version: 1 + status: draft + steps: [] + blocks: + - id: ask + outputs: [{ k: answer, label: Answer, t: text }] + steps: + - id: guard + use: core.try + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + handler: + - { id: fallback, use: core.return, with: { answer: "no" } } + expect: + - { code: BLOCK_PATH_WITHOUT_RETURN, blocks: publish, blockId: ask } + + - name: nothing after a try that returns on both regions can run + definition: + id: wf + name: W + version: 1 + status: draft + steps: [] + blocks: + - id: ask + outputs: [{ k: answer, label: Answer, t: text }] + steps: + - id: guard + use: core.try + steps: + - { id: ret, use: core.return, with: { answer: "yes" } } + handler: + - { id: fallback, use: core.return, with: { answer: "no" } } + - { id: after, use: component.email.send, with: { to: "me@dane.dev" } } + expect: + - { code: STEP_AFTER_RETURN, blocks: publish, stepId: after, blockId: ask } + + - name: a step after a return inside a handler can never run, because the handler is walked too + definition: + id: wf + name: W + version: 1 + status: draft + steps: [] + blocks: + - id: ask + outputs: [{ k: answer, label: Answer, t: text }] + steps: + - id: guard + use: core.try + steps: + - { id: ret, use: core.return, with: { answer: "yes" } } + handler: + - { id: fallback, use: core.return, with: { answer: "no" } } + - { id: never, use: component.email.send, with: { to: "ops@dane.dev" } } + expect: + - { code: STEP_AFTER_RETURN, blocks: publish, stepId: never, blockId: ask } + + - name: a core.return inside a handler on the root board is still outside a block + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - id: guard + use: core.try + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + handler: + - { id: ret, use: core.return } + expect: + - { code: RETURN_OUTSIDE_BLOCK, blocks: publish, stepId: ret } + + - name: recursion through a handler is still recursion, because the call graph walks that region too + definition: + id: wf + name: W + version: 1 + status: draft + steps: [] + blocks: + - id: ask + steps: + - id: guard + use: core.try + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + handler: + # The call that a walk skipping `handler:` would never see — the + # call graph would come out short an edge and the cycle would go + # unreported in whichever language forgot the region. + - { id: again, use: block.ask } + expect: + - { code: BLOCK_RECURSION, blocks: publish, blockId: ask } + + # ---- item, and the list it is resolved through ---------------------------- + + - name: a loop over a list reports nothing, and item is the element it declares + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - { id: fetch, use: component.inbox.fetch } + - id: each + use: core.for_each + with: { list: "{{ steps.fetch.messages }}" } + steps: + - { id: s1, use: component.email.send, with: { to: "{{ steps.each.item.subject }}" } } + expect: [] + + - name: two nested loops each resolve item through their own list, and neither shadows the other + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - { id: fetch, use: component.inbox.threads } + - id: outer + use: core.for_each + with: { list: "{{ steps.fetch.threads }}" } + steps: + - id: inner + use: core.for_each + with: { list: "{{ steps.outer.item.entries }}" } + steps: + - { id: s1, use: component.email.send, with: { to: "{{ steps.inner.item.body }}" } } + expect: [] + + - name: a loop pointed at a number has nothing to iterate + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - { id: fetch, use: component.inbox.fetch } + - id: each + use: core.for_each + with: { list: "{{ steps.fetch.count }}" } + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + expect: + - { code: LOOP_LIST_NOT_A_LIST, blocks: publish, stepId: each, fieldKey: list } + + - name: an inner loop pointed at the outer loop's element rather than a list of them + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - { id: fetch, use: component.inbox.fetch } + - id: outer + use: core.for_each + with: { list: "{{ steps.fetch.messages }}" } + steps: + - id: inner + use: core.for_each + with: { list: "{{ steps.outer.item.subject }}" } + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + expect: + - { code: LOOP_LIST_NOT_A_LIST, blocks: publish, stepId: inner, fieldKey: list } + + - name: a list nothing computes statically is accepted, the way every unknown is + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - { id: fetch, use: component.inbox.fetch } + - id: each + use: core.for_each + with: { list: "{{ json.parse(steps.fetch.count) }}" } + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + expect: [] + + - name: a loop whose list names nothing is a broken reference rather than a wrong type + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - id: each + use: core.for_each + with: { list: "{{ steps.gone.messages }}" } + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + expect: [] + + - name: a loop with no list at all reports the missing field and nothing about its type + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - id: each + use: core.for_each + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + expect: + - { code: FIELD_REQUIRED, blocks: publish, stepId: each, fieldKey: list } + + - name: a loop inside a try's handler resolves item through the handler's own scope + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - { id: fetch, use: component.inbox.fetch } + - id: guard + use: core.try + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + handler: + - id: each + use: core.for_each + with: { list: "{{ steps.fetch.messages }}" } + steps: + - { id: s2, use: component.email.send, with: { to: "{{ steps.each.item.subject }}" } } + expect: [] diff --git a/source/conformance/definition/valid/full.yaml b/source/conformance/definition/valid/full.yaml index 70c3259..c274639 100644 --- a/source/conformance/definition/valid/full.yaml +++ b/source/conformance/definition/valid/full.yaml @@ -1,6 +1,6 @@ # Every section exercised: multiple triggers, a null connection ref, an -# expression var, a condition fork with a fallback branch, a mapping step, and a -# nested loop. +# expression var, a condition fork with a fallback branch, a mapping step, a +# nested loop, and a `core.try` with both of its regions. # # The `when:` below reads `{{ steps.s2.count > 0 }}`, not `{{steps.s2.count}} > 0`. That is a # contract edit rather than a typo fix: the second is a *text* template, and a @@ -96,6 +96,36 @@ steps: - key: busy value: "{{ steps.s2.count > 20 }}" type: boolean + - id: s10 + use: core.try + name: Publish the digest + # The retry policy is ordinary `with:` fields. A count and a delay are + # numbers, and `number` is a mappable field kind — so the argument that + # forced `until` out of `with:` does not reach here, and a manifest field + # gives the form control, the type and the validation for nothing. + with: + attempts: 3 + backoff_ms: 500 + steps: + - id: s11 + use: component.email.send + name: Send the digest + with: + connection: mailbox + to: "{{ var.digest_to }}" + subject: "{{ steps.s8.headline }}" + # The failure is bound as the try's own output and reaches these children + # alone — `{{steps.s10.error}}`. The body cannot read it, because the body is + # what produces it, and neither can s9 below, because whether there was a + # failure at all is decided during a run. + handler: + - id: s12 + use: component.email.send + name: Tell someone + with: + connection: notifier + to: ops@dane.dev + subject: "Digest failed at {{ steps.s10.error.step }}: {{ steps.s10.error.message }}" - id: s9 use: core.end name: End workflow diff --git a/source/conformance/manifest/catalogue.yaml b/source/conformance/manifest/catalogue.yaml index 37e17c7..904e246 100644 --- a/source/conformance/manifest/catalogue.yaml +++ b/source/conformance/manifest/catalogue.yaml @@ -48,6 +48,15 @@ components: - k: subject label: Subject t: text + # The set's one list, and what `t: item` is resolved THROUGH: a + # `core.for_each` pointed here binds an element carrying these members, + # read as `{{steps..item.filename}}`. + - k: attachments + label: Attachments + t: list + of: + - { k: filename, label: Filename, t: text } + - { k: bytes, label: Size, t: number } - kind: component use: component.agent.act name: Run agent @@ -101,12 +110,60 @@ components: icon: /icons/zap.svg blurb: Repeat the steps inside it once per item. fields: - - k: items - label: Items + # `list`, and the name is load-bearing: `item` means "one element of + # whatever THIS key points at", so a reader looking under a different key + # resolves it to nothing and reports no type at all. + - k: list + label: List kind: ref req: true hint: The list to walk. outputs: + # `t: item` rather than `object`. The shape is not in this file — it is one + # element of whatever `list` points at, read off the source output's `of:`. + # Declaring `object` would be a shape nothing verified, and every + # `{{steps..item.}}` would type-check against members the + # manifest never had. - k: item label: Item + t: item + - kind: component + use: core.try + name: Try + group: Built-in + icon: /icons/zap.svg + blurb: Run steps, and fall back to a handler if they fail. + # Wrapping one step is retry; wrapping a region is fallback, so one verb + # serves both. Its two regions are `steps:` and `handler:`, which are + # positions in the document rather than fields — so they are not here. + fields: + # The retry policy IS a set of ordinary fields, deliberately. A condition + # had to leave `with:` because `FIELD_KIND_TYPES` has no mappable boolean; + # a count and a delay are numbers, and `number` is mappable — so the + # argument that moved `until` does not reach these, and a structural key + # here would be a conclusion copied without its reason. + - k: attempts + label: Attempts + kind: number + hint: How many times to run the body before the handler takes over. + - k: backoff_ms + label: Backoff + kind: number + hint: Milliseconds to wait between attempts. + outputs: + # The failure, and the whole of what a try binds. Read as + # `{{steps..error}}` by the handler's children and by nothing else — + # an ordinary output of the container step, which is how core.for_each + # already exposes `item`, and why neither costs a namespace root. + # + # Declared here rather than invented by a runner, because Hatua ships this + # verb (ADR-0014) and this shape is the contract every Host runner fills in. + # `type` is what a core.fork inside the handler branches on, which is why + # error-type matching needs no matcher of its own. + - k: error + label: Error t: object + of: + - { k: message, label: Message, t: text } + - { k: type, label: Type, t: text } + - { k: step, label: Failed step, t: text } diff --git a/source/packages/document/src/round-trip.test.ts b/source/packages/document/src/round-trip.test.ts index ad08f13..f104c0c 100644 --- a/source/packages/document/src/round-trip.test.ts +++ b/source/packages/document/src/round-trip.test.ts @@ -148,6 +148,63 @@ describe('a document with a repeat and a set_var', () => { }) }) +const WITH_A_TRY = `id: wf_publish +name: "Publish the digest" +version: 3 +status: draft + +steps: + - id: fetch + use: component.inbox.fetch + with: + folder: inbox + - id: each + use: core.for_each + # \`item\` is one element of whatever THIS field points at. + with: { list: "{{ steps.fetch.messages }}" } + steps: + - id: guard + use: core.try + with: + attempts: 3 # an ordinary number field, not a structural key + backoff_ms: 500 + steps: + - id: send + use: component.email.send + with: { to: "{{ steps.each.item.sender }}" } + handler: + # Reads the failure the body produced; the body cannot read it back. + - id: warn + use: component.email.send + with: { to: "{{ steps.guard.error.message }}" } +` + +describe('a document with a try and a loop', () => { + it('reproduces it byte for byte, both regions and their comments included', () => { + expect(parseWorkflow(WITH_A_TRY).toString()).toBe(WITH_A_TRY) + }) + + /* + * The two regions are two keys, not two Branches. A Branch's identity is its + * `label` — free text a user renames — so a region spelled as one would have + * its meaning decided by a display name. + */ + it('projects the handler beside the body rather than as a branch', () => { + const doc = parseWorkflow(WITH_A_TRY).toJSON() + const guard = doc.steps[1]?.steps?.[0] + + expect(guard?.use).toBe('core.try') + expect(guard?.steps?.map((step) => step.id)).toEqual(['send']) + expect(guard?.handler?.map((step) => step.id)).toEqual(['warn']) + expect(guard?.branches).toBeUndefined() + }) + + it('projects the retry policy as ordinary field values, which a manifest types', () => { + const guard = parseWorkflow(WITH_A_TRY).toJSON().steps[1]?.steps?.[0] + expect(guard?.with).toEqual({ attempts: 3, backoff_ms: 500 }) + }) +}) + describe('yaml layer fidelity', () => { // Pins the reason @hatua/document keeps the CST rather than the Document API // alone. If a future yaml release makes the AST byte-exact, this test fails diff --git a/source/packages/model/src/blocks.ts b/source/packages/model/src/blocks.ts index fc726f6..014105e 100644 --- a/source/packages/model/src/blocks.ts +++ b/source/packages/model/src/blocks.ts @@ -75,7 +75,10 @@ const declaredSlots = (declarations: readonly Declaration[] | undefined, step: S * Which Blocks a Block reaches directly, in document order. * * Reads the whole Board rather than its top level: a call nested inside a Fork - * branch or a loop body still reaches, which is the entire point of asking. + * branch, a loop body or a `core.try`'s handler still reaches, which is the + * entire point of asking. A region missing from this walk is a region recursion + * can hide in: the call graph comes out short an edge, the cycle is not found, + * and the document publishes. */ export function callsOf(steps: readonly Step[]): string[] { const found: string[] = [] @@ -85,6 +88,7 @@ export function callsOf(steps: readonly Step[]): string[] { if (id !== null) found.push(id) for (const branch of step.branches ?? []) walk(branch.steps) if (step.steps) walk(step.steps) + if (step.handler) walk(step.handler) } } walk(steps) diff --git a/source/packages/model/src/generated/diagnostics.ts b/source/packages/model/src/generated/diagnostics.ts index 101e20f..d264ef2 100644 --- a/source/packages/model/src/generated/diagnostics.ts +++ b/source/packages/model/src/generated/diagnostics.ts @@ -14,6 +14,9 @@ export type DefinitionCode = | 'FORK_NEEDS_TWO_BRANCHES' | 'BRANCH_UNREACHABLE_AFTER' | 'LOOP_HAS_NO_BODY' + | 'TRY_HAS_NO_BODY' + | 'TRY_HAS_NO_HANDLER' + | 'LOOP_LIST_NOT_A_LIST' | 'REPEAT_HAS_NO_CONDITION' | 'VAR_UNKNOWN' | 'BLOCK_UNKNOWN' @@ -65,6 +68,21 @@ export const DEFINITION_DIAGNOSTICS: Record { ).not.toEqual([]) }) }) + +/** + * `core.try` and the `item` binding, on the side the rules corpus cannot reach. + * + * The corpus compares diagnostics. What a container BINDS is a type, and a type + * reaches a user through scope and the checker — so it is asserted here, and in + * `sdk/go/slots_test.go` assertion for assertion. + */ + +const listing = (): Manifest[] => [ + { + kind: 'component', + use: 'component.inbox.fetch', + name: 'Fetch inbox', + fields: [], + outputs: [ + { + k: 'messages', + label: 'Messages', + t: 'list', + of: [{ k: 'subject', label: 'Subject', t: 'text' }], + }, + { k: 'count', label: 'Count', t: 'number' }, + ], + }, + { + kind: 'component', + use: 'core.for_each', + name: 'For each', + fields: [{ k: 'list', label: 'List', kind: 'ref', req: true }], + outputs: [{ k: 'item', label: 'Item', t: 'item' }], + }, + { + kind: 'component', + use: 'core.try', + name: 'Try', + fields: [], + outputs: [ + { + k: 'error', + label: 'Error', + t: 'object', + of: [{ k: 'message', label: 'Message', t: 'text' }], + }, + ], + }, + { kind: 'component', use: 'component.email.send', name: 'Send', fields: [], outputs: [] }, +] + +const pathsIn = (scope: readonly { path: string }[]) => scope.map((entry) => entry.path) + +describe('what a core.try binds, and for whom', () => { + const TRIED = doc({ + steps: [ + { id: 'before', use: 'component.email.send' }, + { + id: 'guard', + use: 'core.try', + steps: [{ id: 'body', use: 'component.email.send' }], + handler: [{ id: 'rescue', use: 'component.email.send' }], + }, + { id: 'after', use: 'component.email.send' }, + ], + }) + + it('is in scope for a handler’s children, and is the failure it is handling', () => { + const scope = scopeFor(TRIED, { board: null, id: 'rescue' }, listing()) + expect(pathsIn(scope)).toEqual(['steps.before', 'steps.guard']) + expect( + validate('{{ steps.guard.error.message }}', 'text', { + scope, + functions: coreFunctions(), + }), + ).toEqual([]) + }) + + /** + * The body is what PRODUCES the failure, so a body Step reading it would be + * reading a value that cannot exist where it stands. + */ + it('is out of scope inside the body', () => { + const scope = scopeFor(TRIED, { board: null, id: 'body' }, listing()) + expect(pathsIn(scope)).toEqual(['steps.before']) + }) + + /** + * Past the try, whether there was a failure at all is a run-time fact — so + * offering it would be the intersection-over-paths problem arriving through a + * different door. + */ + it('is out of scope for a Step after the try', () => { + const scope = scopeFor(TRIED, { board: null, id: 'after' }, listing()) + expect(pathsIn(scope)).toEqual(['steps.before']) + }) + + /** + * The two regions are siblings, which is what a Fork's branches already are. + * Which of the body's Steps completed before the failure is not a property of + * the document. + */ + it('does not let a handler’s children read the body’s Steps, or the reverse', () => { + const scope = scopeFor(TRIED, { board: null, id: 'rescue' }, listing()) + expect(pathsIn(scope)).not.toContain('steps.body') + expect(pathsIn(scopeFor(TRIED, { board: null, id: 'body' }, listing()))).not.toContain( + 'steps.rescue', + ) + }) +}) + +describe('what a core.for_each binds', () => { + const looping = (list: string): WorkflowDefinition => + doc({ + steps: [ + { id: 'fetch', use: 'component.inbox.fetch' }, + { + id: 'each', + use: 'core.for_each', + with: { list: `{{ ${list} }}` }, + steps: [{ id: 's1', use: 'component.email.send' }], + }, + ], + }) + + it('is one element of the list its `list` names, with the members the source declared', () => { + const document = looping('steps.fetch.messages') + expect(loopElementType(document, null, document.steps[1] as Step, listing())).toEqual({ + type: 'object', + members: { subject: { type: 'text' } }, + }) + }) + + it('reaches the checker, so a member of an item type-checks and a wrong type does not', () => { + const document = looping('steps.fetch.messages') + const scope = scopeFor(document, { board: null, id: 's1' }, listing()) + + expect( + validate('{{ steps.each.item.subject }}', 'text', { scope, functions: coreFunctions() }), + ).toEqual([]) + expect( + validate('{{ steps.each.item.subject }}', 'number', { scope, functions: coreFunctions() }), + ).not.toEqual([]) + }) + + /** + * The whole reason the binding is an output of the container rather than a + * bare token: two loops are two Step ids, so nesting needs no shadowing rule + * and there is nothing for an inner loop to hide. + */ + it('is resolved per loop, so two nested loops do not shadow each other', () => { + const nested = doc({ + steps: [ + { id: 'fetch', use: 'component.inbox.threads' }, + { + id: 'outer', + use: 'core.for_each', + with: { list: '{{ steps.fetch.threads }}' }, + steps: [ + { + id: 'inner', + use: 'core.for_each', + with: { list: '{{ steps.outer.item.entries }}' }, + steps: [{ id: 's1', use: 'component.email.send' }], + }, + ], + }, + ], + }) + const manifests: Manifest[] = [ + ...listing(), + { + kind: 'component', + use: 'component.inbox.threads', + name: 'Threads', + fields: [], + outputs: [ + { + k: 'threads', + label: 'Threads', + t: 'list', + of: [ + { + k: 'entries', + label: 'Entries', + t: 'list', + of: [{ k: 'body', label: 'Body', t: 'text' }], + }, + ], + }, + ], + }, + ] + + const scope = scopeFor(nested, { board: null, id: 's1' }, manifests) + expect( + validate('{{ steps.inner.item.body }}', 'text', { scope, functions: coreFunctions() }), + ).toEqual([]) + // The outer loop's element is still reachable and still its own shape — an + // inner `item` hides nothing, because the two live under different Step ids. + expect( + validate('{{ steps.outer.item.entries }}', 'list', { scope, functions: coreFunctions() }), + ).toEqual([]) + }) + + /** + * Null rather than a guess. `item` then stays `item`, which the checker treats + * as matching anything — the honest answer where `object` would be a shape + * nothing declared, and where the wrongness is reported by + * LOOP_LIST_NOT_A_LIST rather than smuggled into a type. + */ + it('is nothing when the list names something that is not one', () => { + const document = looping('steps.fetch.count') + expect(loopElementType(document, null, document.steps[1] as Step, listing())).toBeNull() + }) + + it('is nothing when the list is not a plain Reference, or names nothing at all', () => { + const computed = looping('json.parse(steps.fetch.count)') + expect(loopElementType(computed, null, computed.steps[1] as Step, listing())).toBeNull() + + const gone = looping('steps.gone.messages') + expect(loopElementType(gone, null, gone.steps[1] as Step, listing())).toBeNull() + }) +}) diff --git a/source/packages/model/src/scope.ts b/source/packages/model/src/scope.ts index e30a893..8a11fd4 100644 --- a/source/packages/model/src/scope.ts +++ b/source/packages/model/src/scope.ts @@ -1,4 +1,4 @@ -import type { TypeNode } from '@hatua/expressions' +import { elementOf, sourceReference, type TypeNode } from '@hatua/expressions' import type { Block, ContextKey, @@ -11,8 +11,16 @@ import type { } from '@hatua/schema' import { TRIGGER_BUILTIN } from '@hatua/schema' import { blockIdOf, blockOf } from './blocks' -import { MAPPING_VERB, mapEntries, variableType } from './slots' -import { type BoardId, boardOf, type StepRef } from './tree' +import { + FOR_EACH_LIST_FIELD, + FOR_EACH_VERB, + ITEM_BINDING, + MAPPING_VERB, + mapEntries, + TRY_VERB, + variableType, +} from './slots' +import { type BoardId, boardOf, own, type StepRef, stepKey } from './tree' /** * What a step may reference. The reference tree is built from this, which is @@ -71,17 +79,44 @@ export function upstreamOf(doc: WorkflowDefinition, ref: StepRef): Step[] { return board ? (collectUpstream(board.steps, ref.id, []) ?? []) : [] } +/** + * The walk, and the one verb it treats as more than a container. + * + * A `core.try` has two child regions, and they are siblings — so the body + * cannot see the handler and the handler cannot see the body's Steps, with no + * code saying so. That is the same rule that keeps a Fork's branches out of + * each other's scope, and it is the right one for the right reason: the body + * failed *somewhere*, and which of its Steps completed before it did is not a + * fact the document holds. Offering them would make scope an intersection over + * paths, which is the analysis ADR-0013 refuses edges in order to avoid. + * + * What IS special is the try Step itself. It appears in the upstream list only + * for Steps inside its `handler`, because that is where its binding means + * something: `{{steps..error}}` is the failure the handler is handling. + * Its body cannot read it — the body is what produces it — and neither can a + * Step after the try, because whether there was a failure at all is decided + * during a run and not in the file. + */ function collectUpstream(steps: readonly Step[], id: string, ancestors: Step[]): Step[] | null { const earlier: Step[] = [] for (const step of steps) { if (step.id === id) return [...ancestors, ...earlier] - const nested = [...(step.branches ?? []).map((b) => b.steps), step.steps ?? []] - for (const children of nested) { - const hit = collectUpstream(children, id, [...ancestors, ...earlier, step]) + const above = [...ancestors, ...earlier] + const outside = step.use === TRY_VERB ? above : [...above, step] + + const regions: readonly (readonly [readonly Step[], Step[]])[] = [ + ...(step.branches ?? []).map((branch) => [branch.steps, outside] as const), + [step.steps ?? [], outside] as const, + // The handler, and the only place the try itself is in scope. + [step.handler ?? [], [...above, step]] as const, + ] + for (const [children, visible] of regions) { + const hit = collectUpstream(children, id, visible) if (hit) return hit } - earlier.push(step) + + if (step.use !== TRY_VERB) earlier.push(step) } return null } @@ -219,6 +254,27 @@ export function scopeFor( ref: StepRef, manifests: readonly Manifest[] = [], context: readonly ContextKey[] = [], +): ScopeEntry[] { + return scopeAt(doc, ref, manifests, context, new Set()) +} + +/** + * `scopeFor`, plus the set of loops already being resolved. + * + * `item` is typed by reading the loop's own `list` field, which means typing an + * expression against the loop step's scope — so building one Step's scope can + * ask for another's. The walk terminates on its own, because a Step's upstream + * is always strictly earlier in the tree than the Step itself and a loop can + * therefore never be its own. `resolving` is not that argument: it is the guard + * for a document where two Steps share an id, which the schema permits into a + * file and `STEP_ID_DUPLICATE` reports rather than refuses. + */ +function scopeAt( + doc: WorkflowDefinition, + ref: StepRef, + manifests: readonly Manifest[], + context: readonly ContextKey[], + resolving: ReadonlySet, ): ScopeEntry[] { const byUse = new Map(manifests.map((manifest) => [manifest.use, manifest])) @@ -229,12 +285,89 @@ export function scopeFor( path: `steps.${step.id}`, kind: 'step', label: step.name ?? step.id, - type: stepOutputType(doc, step, byUse.get(step.use)), + type: stepOutputType(doc, ref.board, step, byUse.get(step.use), { + manifests, + context, + resolving, + }), }), ), ] } +/** + * The type one path names, read out of a scope, or null when nothing declares + * it. + * + * Longest prefix first, because a scope path is dotted and is one entry rather + * than two: `steps.s2` is an entry and `steps` is not, so `steps.s2.messages` + * has to try three segments before two. That is the same rule `validate.ts` + * walks a Member chain by, restated here for a caller that has a path and no + * expression — reading a *declared* type is not checking one, and reaching for + * the checker would mean manufacturing diagnostics nobody asked for in order to + * throw them away. + */ +export function typeAtPath(scope: readonly ScopeEntry[], path: string): TypeNode | null { + const segments = path.split('.') + for (let take = segments.length; take > 0; take--) { + const entry = scope.find((candidate) => candidate.path === segments.slice(0, take).join('.')) + if (!entry) continue + + let node: TypeNode = entry.type + for (const name of segments.slice(take)) { + // `Object.hasOwn`, for the reason `own` exists: a member called + // `constructor` would otherwise resolve off `Object.prototype` and give a + // shape nothing declared, which Go — having no prototype — would not. + if (!node.members || !Object.hasOwn(node.members, name)) return null + node = node.members[name] as TypeNode + } + return node + } + return null +} + +/** + * What one element of a `core.for_each`'s list is, or null when the document + * does not say. + * + * This is the whole of `t: item`. The loop's `list` is a `ref` field, so its + * declared type is `unknown` and the ordinary Slot check learns nothing from + * it; the shape is one level below whatever it points at, which is exactly the + * `of:` the source output declared. Null when `list` is missing, is not a plain + * Reference, names nothing, or names something that is not a list — and null + * means `item` stays `item`, which the checker treats as matching anything. + * Guessing `object` instead would be a shape nothing declared. + */ +export function loopElementType( + doc: WorkflowDefinition, + board: BoardId, + step: Step, + manifests: readonly Manifest[] = [], + context: readonly ContextKey[] = [], + resolving: ReadonlySet = new Set(), +): TypeNode | null { + const template = own(step.with as Record | undefined, FOR_EACH_LIST_FIELD) + if (typeof template !== 'string') return null + + const path = sourceReference(template) + if (path === null) return null + + const key = stepKey({ board, id: step.id }) + if (resolving.has(key)) return null + + const scope = scopeAt( + doc, + { board, id: step.id }, + manifests, + context, + new Set([...resolving, key]), + ) + const node = typeAtPath(scope, path) + if (!node || node.type !== 'list') return null + + return elementOf(node) +} + /** * What a Block's outputs are, as a scope entry's type at the call site. * @@ -300,15 +433,48 @@ function variableToType(variable: Variable): TypeNode { */ function stepOutputType( doc: WorkflowDefinition, + board: BoardId, step: Step, manifest: Manifest | undefined, + through: { + manifests: readonly Manifest[] + context: readonly ContextKey[] + resolving: ReadonlySet + }, ): TypeNode { if (step.use === MAPPING_VERB) return mappingOutputType(step) const called = blockIdOf(step.use) if (called !== null) return blockOutputType(blockOf(doc, called)) - return outputsToType(manifest?.outputs ?? []) + const declared = outputsToType(manifest?.outputs ?? []) + if (step.use !== FOR_EACH_VERB) return declared + + /* + * A loop's binding, and the one output whose type is not in the manifest. + * + * `item` is substituted here rather than in `outputsToType` because it is a + * property of the STEP and not of the declaration: two `core.for_each` steps + * share one manifest and iterate two different lists. Only a top-level output + * is substituted — `item` nested inside another output's `of:` would be a + * loop's element appearing as a member of something that is not the loop, + * which no manifest can mean. + */ + const element = loopElementType( + doc, + board, + step, + through.manifests, + through.context, + through.resolving, + ) + if (!element) return declared + + const members: Record = Object.create(null) + for (const [key, node] of Object.entries(declared.members ?? {})) { + members[key] = node.type === ITEM_BINDING ? element : node + } + return { type: 'object', members } } function mappingOutputType(step: Step): TypeNode { diff --git a/source/packages/model/src/slots.ts b/source/packages/model/src/slots.ts index 7af2558..a87ba89 100644 --- a/source/packages/model/src/slots.ts +++ b/source/packages/model/src/slots.ts @@ -114,6 +114,46 @@ export const FORK_VERB = 'core.fork' /** The verb that writes one of its Board's variables. */ export const SET_VAR_VERB = 'core.set_var' +/** + * The verb that protects a region and falls back to a handler. + * + * The one container with two child regions: a body under `steps:` and a handler + * under `handler:`. Wrapping one Step is retry, wrapping a region is fallback, + * so one verb serves both (ADR-0013). + * + * Its retry policy — how many attempts, how long to wait — sits in `with:` as + * ordinary manifest fields, and deliberately NOT in a structural key. `until` + * had to leave `with:` because `FIELD_KIND_TYPES` has no mappable boolean, so a + * condition there would have type-checked as text. An attempt count is a number + * and `number` IS a mappable field kind, so the argument that moved `until` does + * not reach here at all — following it anyway would be copying a conclusion + * without its reason, and would cost a structural key, a diagnostic and a form + * control that the manifest already gives for nothing. + */ +export const TRY_VERB = 'core.try' + +/** + * The field a `core.for_each` iterates, and the one `item` is resolved through. + * + * A constant rather than a literal at three call sites, because it is a name two + * languages and one manifest have to agree on: `item` means "one element of + * whatever THIS key points at", so a reader looking under a different key + * resolves `item` to nothing and reports no type at all. + */ +export const FOR_EACH_LIST_FIELD = 'list' + +/** + * The output key a container binds for the children it owns. + * + * Both are ordinary manifest outputs of the container Step, read as + * `{{steps..}}`. That is the whole binding mechanism, and it + * is one mechanism rather than two: ADR-0014 closed the path roots so that a + * structural idea could not take a bare word away from users, and a Step id is + * already one segment below `steps.`. Two nested loops cannot shadow each other, + * because two Steps cannot share an id on one Board. + */ +export const ITEM_BINDING = 'item' + /** * The Slot a `core.repeat`'s `until` resolves into. * diff --git a/source/packages/model/src/tree.ts b/source/packages/model/src/tree.ts index 52d1af6..aff8051 100644 --- a/source/packages/model/src/tree.ts +++ b/source/packages/model/src/tree.ts @@ -67,12 +67,21 @@ export function boardOf(doc: WorkflowDefinition, id: BoardId): Board | undefined return undefined } -/** Depth-first walk of every step in one tree, parents before children. */ +/** + * Depth-first walk of every step in one tree, parents before children. + * + * Every region a container owns is walked here and nowhere else: a Fork's + * branches, a loop body, and a `core.try`'s handler. A region this forgets is a + * region no rule ever sees — the validator reports nothing about it, silently, + * which is the same failure as a validator that only ever looked at the root + * Board. + */ export function* walkSteps(steps: readonly Step[]): Generator { for (const step of steps) { yield step for (const branch of step.branches ?? []) yield* walkSteps(branch.steps) if (step.steps) yield* walkSteps(step.steps) + if (step.handler) yield* walkSteps(step.handler) } } diff --git a/source/packages/model/src/validity.ts b/source/packages/model/src/validity.ts index 6949204..a62df2b 100644 --- a/source/packages/model/src/validity.ts +++ b/source/packages/model/src/validity.ts @@ -1,8 +1,17 @@ +import { sourceReference, type TypeNode } from '@hatua/expressions' import type { Block, Declaration, Manifest, Step, WorkflowDefinition } from '@hatua/schema' import { blockIdOf, blockOf, cyclicBlocks, RETURN_VERB } from './blocks' import type { Diagnostic } from './connections' import { DEFINITION_DIAGNOSTICS, type DefinitionCode } from './generated/diagnostics' -import { FOR_EACH_VERB, FORK_VERB, REPEAT_VERB, SET_VAR_VERB } from './slots' +import { scopeFor, typeAtPath } from './scope' +import { + FOR_EACH_LIST_FIELD, + FOR_EACH_VERB, + FORK_VERB, + REPEAT_VERB, + SET_VAR_VERB, + TRY_VERB, +} from './slots' import { type BoardId, boards, own, stepKey, varsOn, walkDocument, walkSteps } from './tree' /** @@ -240,8 +249,12 @@ export function unknownComponents(doc: WorkflowDefinition, manifests: ManifestIn * cannot express them: `core.fork`'s Branches and `core.for_each`'s body are * positions in the document, not fields under `with:`. */ -export function malformedContainers(doc: WorkflowDefinition): Diagnostic[] { +export function malformedContainers( + doc: WorkflowDefinition, + manifests: ManifestIndex = new Map(), +): Diagnostic[] { const out: Diagnostic[] = [] + const catalogue = [...manifests.values()] for (const { step, board } of walkDocument(doc)) { const subject: Partial = { stepId: step.id, ...boardOn(board) } @@ -285,6 +298,46 @@ export function malformedContainers(doc: WorkflowDefinition): Diagnostic[] { out.push(raise('REPEAT_HAS_NO_CONDITION', subject)) } + /* + * A `core.try` is two regions, so it is two codes. + * + * Not LOOP_HAS_NO_BODY, which reads "this loop repeats nothing" — the right + * sentence about a for_each and the wrong one about a verb that is not a + * loop. The Fork's pair settled this shape already: separate codes where the + * missing half differs, because the sentence that helps differs with it. + */ + if (step.use === TRY_VERB) { + if ((step.steps ?? []).length === 0) out.push(raise('TRY_HAS_NO_BODY', subject)) + if ((step.handler ?? []).length === 0) out.push(raise('TRY_HAS_NO_HANDLER', subject)) + } + + /* + * The rule that makes `t: item` observable. + * + * `list` is a `ref` field and `FIELD_KIND_TYPES` maps `ref` to `unknown`, so + * the ordinary Slot check accepts anything written there — which is how a + * loop pointed at a number type-checks clean while `item` quietly resolves + * to nothing and matches everything downstream. Only a statically-KNOWN + * conflict is reported, matching the lattice everywhere else: an expression + * with no static type is accepted with the run-time check every unknown + * gets. + */ + if (step.use === FOR_EACH_VERB) { + const named = loopListType(doc, board, step, catalogue) + if (named && named.type !== 'list' && named.type !== 'unknown' && named.type !== 'item') { + out.push( + raise( + 'LOOP_LIST_NOT_A_LIST', + { ...subject, fieldKey: FOR_EACH_LIST_FIELD }, + { + name: listReferenceOf(step) ?? FOR_EACH_LIST_FIELD, + actual: named.type, + }, + ), + ) + } + } + if (step.use === SET_VAR_VERB) { const key = own((step.with ?? {}) as Record, 'key') // A missing key is FIELD_REQUIRED's to report. Resolving `undefined` @@ -309,6 +362,33 @@ const SET_VAR_FIELDS: readonly (readonly [string, string])[] = [ ['value', 'Value'], ] +/** The path a loop's `list` names, when it names exactly one and nothing else. */ +function listReferenceOf(step: Step): string | null { + const template = own(step.with as Record | undefined, FOR_EACH_LIST_FIELD) + return typeof template === 'string' ? sourceReference(template) : null +} + +/** + * The declared type of what a loop's `list` names, or null when the document + * does not say. + * + * Deliberately the same read `loopElementType` performs, one step short of + * taking the element: the diagnostic and the binding must agree about which + * lists are lists, and two readings of one field are two answers waiting to + * disagree — a loop reported as iterating a number while `item` resolved + * anyway, or the reverse. + */ +function loopListType( + doc: WorkflowDefinition, + board: BoardId, + step: Step, + manifests: readonly Manifest[], +): TypeNode | null { + const path = listReferenceOf(step) + if (path === null) return null + return typeAtPath(scopeFor(doc, { board, id: step.id }, manifests), path) +} + /** * Whether a step list, read from its own root level, always reaches a return. * @@ -334,6 +414,21 @@ function alwaysReturns(steps: readonly Step[]): boolean { // unconditional. if (step.use === REPEAT_VERB) return alwaysReturns(step.steps ?? []) + /* + * A `core.try` discharges the obligation only when BOTH regions do. + * + * The body always runs, so on its own it would look like a repeat. But a + * failure part-way through the body is exactly what a try exists to admit, + * and that path leaves the body without finishing it and enters the handler + * instead. So the guarantee is the conjunction: every path out of a try goes + * through the body OR through the handler, and a region that may skip its + * return leaves one of them open. That is the Fork's all-branches reasoning, + * asked of two regions where one of them is conditional. + */ + if (step.use === TRY_VERB) { + return alwaysReturns(step.steps ?? []) && alwaysReturns(step.handler ?? []) + } + if (step.use !== FORK_VERB) return false const branches = step.branches ?? [] @@ -442,12 +537,20 @@ export function blockRules(doc: WorkflowDefinition): Diagnostic[] { return out } -/** Every step list in a tree — the root, each Branch's, and each loop body's. */ +/** + * Every step list in a tree — the root, each Branch's, each loop body's, and + * each `core.try`'s handler. + * + * A region missing from here is a region STEP_AFTER_RETURN never looks at, so a + * Step sitting after a `core.return` inside a handler would be reported by + * neither language and published by both. + */ function* stepLists(steps: readonly Step[]): Generator { yield steps for (const step of steps) { for (const branch of step.branches ?? []) yield* stepLists(branch.steps) if (step.steps) yield* stepLists(step.steps) + if (step.handler) yield* stepLists(step.handler) } } @@ -501,7 +604,7 @@ export function validateDefinition(doc: WorkflowDefinition, manifests: ManifestI const all = [ ...unknownComponents(doc, manifests), ...missingRequiredFields(doc, manifests), - ...malformedContainers(doc), + ...malformedContainers(doc, manifests), ...blockRules(doc), ] diff --git a/source/packages/react/src/layouts/StepList.stories.tsx b/source/packages/react/src/layouts/StepList.stories.tsx index 048a0b8..6e13ed7 100644 --- a/source/packages/react/src/layouts/StepList.stories.tsx +++ b/source/packages/react/src/layouts/StepList.stories.tsx @@ -98,6 +98,19 @@ steps: - id: s11 use: component.email.archive name: "Archive" + - id: s12 + use: core.try + name: "Publish the digest" + with: + attempts: 3 + steps: + - id: s13 + use: component.email.send + name: "Send it" + handler: + - id: s14 + use: component.chat.post + name: "Say it failed" ` const EMPTY = `id: wf_empty\nname: "Nothing yet"\nversion: 1\nstatus: draft\nsteps: []\n` @@ -162,10 +175,16 @@ type Story = StoryObj export const Flat: Story = { parameters: wired(serving(SIMPLE)) } /** - * Forks, a fallback Branch, a nested loop and an empty Branch — the whole - * vocabulary in one document. `if` / `else if` / `else` for a condition fork - * and `and` for a parallel one, read from whether any Branch carries `when`, - * because the schema has no mode field. + * Forks, a fallback Branch, a nested loop, an empty Branch and a `core.try` with + * both of its regions — the whole vocabulary in one document. `if` / `else if` / + * `else` for a condition fork and `and` for a parallel one, read from whether + * any Branch carries `when`, because the schema has no mode field. + * + * The try is the one Step here with TWO child regions, and the chips say which + * is which: `try` over the protected body and `on failure` over the handler. + * `steps:` holds a loop's children and a try's body alike, so the word comes + * from the verb — reading "loop" over the Steps a try is protecting would name + * the wrong control flow. */ export const DeepTree: Story = { parameters: wired(serving(DEEP)) } diff --git a/source/packages/react/src/layouts/StepList.test.tsx b/source/packages/react/src/layouts/StepList.test.tsx index 32ecf06..c9aa682 100644 --- a/source/packages/react/src/layouts/StepList.test.tsx +++ b/source/packages/react/src/layouts/StepList.test.tsx @@ -598,6 +598,67 @@ describe('when the Host rejects a write', () => { }) }) +/* + * Its own document rather than a `core.try` added to SOURCE: the tests above + * assert exact row lists and exact drag destinations, so a Step appended to the + * shared fixture changes what they are about. + */ +const TRIED = `id: wf_morning +name: "Morning inbox triage" +version: 4 +status: draft + +steps: + - id: s1 + use: core.try + name: "Publish the digest" + steps: + - id: s2 + use: component.email.send + name: "Send it" + handler: + - id: s3 + use: component.chat.post + name: "Say it failed" +` + +describe('a core.try draws two regions', () => { + /* + * The one Step with two child regions, so the one place the tree has to say + * which is which. `steps:` holds a loop's children and a try's body alike, so + * the word comes from the verb — "loop" over the Steps a try is protecting + * would name the wrong control flow. + */ + it('labels the body `try` and the handler `on failure`, rather than calling either a loop', async () => { + mount(host(TRIED)) + await screen.findByText('Publish the digest') + + const card = rowFor('Publish the digest') + const chips = [...card.querySelectorAll('span')] + .map((one) => one.textContent) + .filter((text) => text === 'try' || text === 'on failure' || text === 'loop') + + expect(chips).toEqual(['try', 'on failure']) + }) + + it('draws a Step from each region, so neither is a region nothing renders', async () => { + mount(host(TRIED)) + expect(await screen.findByText('Send it')).toBeDefined() + expect(screen.getByText('Say it failed')).toBeDefined() + }) + + /* + * The one Step whose expanded height is not what its count implies: a body + * count alone reads as the whole of it, on a card that opens into two + * regions. + */ + it('says it has a handler in its summary, which a body count alone would hide', async () => { + mount(host(TRIED)) + await screen.findByText('Publish the digest') + expect(rowFor('Publish the digest').textContent).toContain('core.try · 1 step · handler') + }) +}) + describe('landmarks', () => { it('nests the tree so a screen reader hears three top-level Steps, not eleven rows', async () => { // A flat list with a computed indent looks identical and says the wrong diff --git a/source/packages/react/src/layouts/StepList.tsx b/source/packages/react/src/layouts/StepList.tsx index 89c30f1..f88b58c 100644 --- a/source/packages/react/src/layouts/StepList.tsx +++ b/source/packages/react/src/layouts/StepList.tsx @@ -1,4 +1,4 @@ -import type { Diagnostic } from '@hatua/model' +import { type Diagnostic, TRY_VERB } from '@hatua/model' import type { Branch, Step } from '@hatua/schema' import { type EditingState, @@ -446,15 +446,38 @@ function Sequence({ steps, scope, at, ...handlers }: SequenceProps) { {open && step.steps ? (
- loop + {bodyKeywordFor(step)}
) : null} + + {/* + A `core.try`'s second region, drawn whenever the key is present — + the same rule the body above follows. Its own region rather than + a Branch: a Branch's identity is its label, which is free text a + user renames, and a region the user could rename out of existence + is not a region. + + Rendered only for a try. A `handler:` on any other verb means + nothing, and drawing one would put a region on screen that no + rule and no runner reads. + */} + {open && step.use === TRY_VERB && step.handler ? ( +
+ on failure + +
+ ) : null} void onRemove: (id: string) => void }) { - const container = Boolean(step.branches?.length || step.steps) + const container = Boolean(step.branches?.length || step.steps || step.handler) // Hovering anywhere on the row explains the marker, rather than asking anyone // to find a 3px edge with a pointer. @@ -723,13 +746,36 @@ function keywordFor(branches: readonly Branch[], index: number): string { return 'else' } -/** `core.fork · 2 branches` — the verb, and what makes this Step structural. */ +/** + * What the chip over a container's first region says. + * + * `steps:` is one key holding two different ideas — a loop's body and a try's + * protected region — so the word comes from the verb rather than from the key. + * Reading "loop" over the steps a try is protecting would name the wrong + * control flow. + */ +const bodyKeywordFor = (step: Step) => (step.use === TRY_VERB ? 'try' : 'loop') + +/** The same word inside the sentence a screen reader hears on an insert point. */ +const regionNoun = (step: Step) => (step.use === TRY_VERB ? 'body' : 'loop') + +/** + * `core.fork · 2 branches` — the verb, and what makes this Step structural. + * + * A `core.try` says `· handler` as well as its body count, because it is the one + * Step whose expanded height is not what its count implies: the body count alone + * reads as the whole of it, and a reader scanning a collapsed list would see + * "1 step" on a card that opens into two regions. Absent when there is no + * handler — which is a diagnostic of its own, so the summary says nothing about + * it and lets the marker do that. + */ function metaFor(step: Step): string { const count = step.branches?.length if (count) return `${step.use} · ${count} ${count === 1 ? 'branch' : 'branches'}` const nested = step.steps?.length - if (nested !== undefined) return `${step.use} · ${nested} ${nested === 1 ? 'step' : 'steps'}` + if (nested === undefined) return step.use - return step.use + const body = `${step.use} · ${nested} ${nested === 1 ? 'step' : 'steps'}` + return step.handler?.length ? `${body} · handler` : body } diff --git a/source/packages/schema/src/generated/component.ts b/source/packages/schema/src/generated/component.ts index 06fda51..f3793c0 100644 --- a/source/packages/schema/src/generated/component.ts +++ b/source/packages/schema/src/generated/component.ts @@ -130,7 +130,8 @@ export const output = z.strictObject({ */ label: z.string().min(1), /** - * `item` is the for-each escape hatch: the shape is not known statically, so it is resolved by following the loop's `list` reference to the source output's `of`. + * `item` is the for-each escape hatch, and the one type whose meaning depends on the step declaring it: a `core.for_each`'s `item` output is whatever its `list` field points at, one element deep. It is resolved by reading that field as a Reference, typing the path against the loop step's own scope, and taking the element shape of the `list` it names — which is the `of:` the source output declared. + * Declaring it anywhere else is a promise nothing can keep. A component that is not a loop has no `list` to follow, so its `item` resolves to nothing and the checker treats it as `unknown` — which switches the type gate off for that output rather than narrowing it. That is also why a Block's declaration and a Board's variable refuse `item` outright: neither is the output of anything. */ t: z.enum(['text', 'number', 'boolean', 'datetime', 'object', 'list', 'item']), /** diff --git a/source/packages/schema/src/generated/definition.ts b/source/packages/schema/src/generated/definition.ts index 5790646..e9ec166 100644 --- a/source/packages/schema/src/generated/definition.ts +++ b/source/packages/schema/src/generated/definition.ts @@ -148,8 +148,9 @@ export const step = z.strictObject({ }, /** * The manifest verb. Its root says who declares it and there are only three: `core.` is Hatua's, `component.` is the Host's, `block.` names a Block in this document. Nothing sits at the root itself, so a Host may declare `component.block.render` and collide with nothing — see ADR-0014. - * Hatua treats most verbs as opaque, but interprets five structurally: `core.fork` creates branches, `core.for_each` nests and exposes `item`, `core.repeat` nests and carries an `until:` condition, `core.map` derives its outputs from its own `entries` field rather than from its manifest, and `core.set_var` writes one of the Board's `vars` and is typed by that var's declaration rather than by a manifest field. - * The three nesting verbs drive reference scope and derived layout; `core.map` drives reference scope alone; `core.set_var` drives neither and is here because its `value` is a Slot no manifest can type. + * Hatua treats most verbs as opaque, but interprets six structurally: `core.fork` creates branches, `core.for_each` nests and exposes `item`, `core.repeat` nests and carries an `until:` condition, `core.try` nests TWICE — a body under `steps:` and a fallback under `handler:` — `core.map` derives its outputs from its own `entries` field rather than from its manifest, and `core.set_var` writes one of the Board's `vars` and is typed by that var's declaration rather than by a manifest field. + * The four nesting verbs drive reference scope and derived layout; `core.map` drives reference scope alone; `core.set_var` drives neither and is here because its `value` is a Slot no manifest can type. + * Two of them BIND a name for the children they own, and both bind it the same way: as an output of the container Step itself, read as `{{steps..}}`. A loop's `item` and a try's `error` therefore cost no namespace root and no bare token — ADR-0014 closed the roots precisely so a structural idea could not take a word away from users — and two nested containers cannot shadow each other, because two Steps cannot share an id on one Board. * `core.map` is the one component whose outputs a manifest cannot declare, because they are whatever the user named. Its `with.entries` is a list of `{key, value, type}`, and a downstream step addresses them as `{{steps..}}` with the declared type — which is what lets the type checker treat a mapping step exactly like any other. */ use: z.string().min(1), @@ -164,11 +165,21 @@ export const step = z.strictObject({ return z.array(branch).min(1).optional() }, /** - * Loop children, nested directly with no branch wrapper. On `core.for_each` and `core.repeat`. + * Loop children, nested directly with no branch wrapper. On `core.for_each` and `core.repeat`, and a `core.try`'s protected body. */ get steps() { return z.array(step).optional() }, + /** + * A `core.try`'s fallback region, and only meaningful there. The body under `steps:` runs; if it fails, this runs instead of the rest of it. + * A key beside `steps:` rather than a pair of `branches:` under reserved labels. A Branch's identity is its `label`, which is free text the user renames — putting "which region is this" into a string a user edits makes the meaning of the document depend on a display name, and it costs the schema its first reserved word. A key cannot collide with anything a user chooses, because nothing inside a step is user-named. + * The failure is exposed to THESE children and to nothing else, as `{{steps..error}}` — the container's own output, which is how `core.for_each` already exposes `item`. The body cannot see it, because the body is what produces it; a Step after the try cannot either, because whether there was a failure at all is a run-time fact. + * Handler children cannot read the body's Steps. The body failed somewhere, and which of its Steps completed is not a property of the document — offering them would make scope an intersection over paths, which is the exact analysis ADR-0013 refuses edges to avoid. It is the same rule that keeps a Fork's sibling branches out of each other's scope, and it needs no code of its own: the two regions are siblings. + * Error-type matching needs no matcher: a `core.fork` inside the handler branches on `{{steps..error.type}}`. + */ + get handler() { + return z.array(step).optional() + }, /** * A `core.repeat`'s termination condition, and only meaningful there. The body runs, then this is evaluated; false runs it again. So a repeat ALWAYS runs its children at least once — which is what lets one discharge a block's return obligation where a `core.for_each` cannot, and what makes a pre-tested loop expressible as a body that starts with a fork while the reverse costs a duplicated body. * A structural key beside `steps:` rather than a field under `with:`, for the reason a Branch's `when` is one: a manifest field carries a rendering `kind` and no type, so the expected type is recovered from `FIELD_KIND_TYPES` — and that vocabulary cannot express "a Template that must produce a boolean" at all. Under `with:` the condition would type-check as text, which is the whole half of the contract it exists to carry. diff --git a/source/packages/services/src/steps.ts b/source/packages/services/src/steps.ts index 8a6b6f9..80f151a 100644 --- a/source/packages/services/src/steps.ts +++ b/source/packages/services/src/steps.ts @@ -45,6 +45,15 @@ export interface InsertPoint { * own nested `steps`, and absent at the root. */ branchIndex?: number + /** + * Which of a `core.try`'s two regions. Absent means the body under `steps:`, + * which is the same key a loop's children sit under. + * + * A named region rather than a second index, because the two are not a list: + * a try has exactly one body and exactly one handler, and an index would let + * a caller ask for the third one. + */ + region?: 'handler' /** Position among the siblings. The list's length appends. */ index: number } @@ -100,6 +109,7 @@ function* walk( } } if (step.steps) yield* walk(step.steps, [...base, index, 'steps']) + if (step.handler) yield* walk(step.handler, [...base, index, 'handler']) } } @@ -147,9 +157,10 @@ function listPathOf(document: WorkflowDocument, point: InsertPoint): Path { if (!parent) throw new Error(`No Step with id "${point.parentId}"`) const parentPath = [...parent.listPath, parent.index] - return point.branchIndex === undefined - ? [...parentPath, 'steps'] - : [...parentPath, 'branches', point.branchIndex, 'steps'] + if (point.branchIndex !== undefined) { + return [...parentPath, 'branches', point.branchIndex, 'steps'] + } + return [...parentPath, point.region === 'handler' ? 'handler' : 'steps'] } /** @@ -304,6 +315,10 @@ export const stepIn = (steps: readonly Step[], id: string): Step | undefined => const hit = stepIn(step.steps, id) if (hit) return hit } + if (step.handler) { + const hit = stepIn(step.handler, id) + if (hit) return hit + } } return undefined } diff --git a/source/pnpm-lock.yaml b/source/pnpm-lock.yaml index 29e3314..65e2c08 100644 --- a/source/pnpm-lock.yaml +++ b/source/pnpm-lock.yaml @@ -45,6 +45,15 @@ importers: specifier: ^19.2.0 version: 19.2.8(react@19.2.8) devDependencies: + '@hatua/expressions': + specifier: workspace:* + version: link:../../packages/expressions + '@hatua/model': + specifier: workspace:* + version: link:../../packages/model + '@hatua/schema': + specifier: workspace:* + version: link:../../packages/schema '@hatua/sdk': specifier: workspace:* version: link:../../sdk/js @@ -57,6 +66,9 @@ importers: '@vitejs/plugin-react-swc': specifier: ^4.3.3 version: 4.3.3(vite@8.2.1(@types/node@26.2.0)(esbuild@0.28.2)(yaml@2.9.0)) + yaml: + specifier: ^2.8.1 + version: 2.9.0 packages/document: dependencies: diff --git a/source/schemas/component-manifest.schema.yaml b/source/schemas/component-manifest.schema.yaml index 7dc2365..40044f6 100644 --- a/source/schemas/component-manifest.schema.yaml +++ b/source/schemas/component-manifest.schema.yaml @@ -173,8 +173,17 @@ $defs: t: enum: [text, number, boolean, datetime, object, list, item] description: >- - `item` is the for-each escape hatch: the shape is not known statically, so it is resolved - by following the loop's `list` reference to the source output's `of`. + `item` is the for-each escape hatch, and the one type whose meaning depends on the step + declaring it: a `core.for_each`'s `item` output is whatever its `list` field points at, + one element deep. It is resolved by reading that field as a Reference, typing the path + against the loop step's own scope, and taking the element shape of the `list` it names — + which is the `of:` the source output declared. + + Declaring it anywhere else is a promise nothing can keep. A component that is not a loop + has no `list` to follow, so its `item` resolves to nothing and the checker treats it as + `unknown` — which switches the type gate off for that output rather than narrowing it. + That is also why a Block's declaration and a Board's variable refuse `item` outright: + neither is the output of anything. of: type: array description: Shape of each list element or object member. diff --git a/source/schemas/definition-diagnostics.yaml b/source/schemas/definition-diagnostics.yaml index a9deb7d..51e06ca 100644 --- a/source/schemas/definition-diagnostics.yaml +++ b/source/schemas/definition-diagnostics.yaml @@ -72,6 +72,45 @@ codes: two: the mistake is the same and so is the fix, and the message names neither verb. + - code: TRY_HAS_NO_BODY + blocks: publish + message: "This try protects nothing. Add at least one Step to its body." + summary: >- + A `core.try` whose `steps:` is empty. Its own code rather than + LOOP_HAS_NO_BODY, for the reason a fork has two: the sentence that helps + names the region, and "this loop repeats nothing" is the wrong sentence + about a verb that is not a loop. + + - code: TRY_HAS_NO_HANDLER + blocks: publish + message: "This try has no handler, so a failure has nowhere to go. Add a Step to `handler:`." + summary: >- + A `core.try` whose `handler:` is absent or empty. Two codes rather than + one, exactly as a fork's two are: an empty body and an empty handler are + different halves missing, and the fix differs. + + A try with no handler is not "the same steps without protection" — it is a + retry policy with nothing to fall back to, and the region that binds the + failure does not exist, so `{{steps..error}}` resolves nowhere. + + - code: LOOP_LIST_NOT_A_LIST + blocks: publish + message: '"{name}" is {actual}, not a list, so there is nothing to iterate.' + summary: >- + A `core.for_each` whose `list` names a value whose declared type is neither + `list` nor unknown. This is the one rule that makes `t: item` observable: + `list` is a `ref` field, `FIELD_KIND_TYPES` maps `ref` to `unknown`, so the + ordinary Slot check accepts anything written there and `item` then resolves + to nothing while the checker treats it as matching everything. + + Only a statically-KNOWN conflict is reported, matching the type lattice + everywhere else: `{{ json.parse(x) }}` is `unknown` and is accepted with the + run-time check that every unknown gets. + + Publish-blocking rather than edit-blocking: retargeting a reference is + ordinary building, and a loop pointed at the wrong step for ten seconds is + what building one looks like. + - code: REPEAT_HAS_NO_CONDITION blocks: publish message: "This repeat has no condition, so nothing ever ends it. Give it an `until`." @@ -144,6 +183,14 @@ codes: reasoning that keeps sibling branches out of scope, applied to time instead of to paths. + A `core.try` discharges it only when BOTH its regions do. Its body always + runs, which alone would read like a repeat — but a failure part-way through + the body is what a try exists to admit, and that path leaves the body + unfinished and enters the handler instead. So the guarantee is a + conjunction: every path out of a try goes through the body or through the + handler, which is the fork's all-branches reasoning asked of two regions + where one is conditional. + - code: STEP_AFTER_RETURN blocks: publish message: "Nothing after `core.return` can run." diff --git a/source/schemas/workflow-definition.schema.yaml b/source/schemas/workflow-definition.schema.yaml index 78c4a3d..bda997e 100644 --- a/source/schemas/workflow-definition.schema.yaml +++ b/source/schemas/workflow-definition.schema.yaml @@ -288,16 +288,24 @@ $defs: sits at the root itself, so a Host may declare `component.block.render` and collide with nothing — see ADR-0014. - Hatua treats most verbs as opaque, but interprets five structurally: `core.fork` creates + Hatua treats most verbs as opaque, but interprets six structurally: `core.fork` creates branches, `core.for_each` nests and exposes `item`, `core.repeat` nests and carries an - `until:` condition, `core.map` derives its outputs from its own `entries` field rather - than from its manifest, and `core.set_var` writes one of the Board's `vars` and is typed - by that var's declaration rather than by a manifest field. + `until:` condition, `core.try` nests TWICE — a body under `steps:` and a fallback under + `handler:` — `core.map` derives its outputs from its own `entries` field rather than from + its manifest, and `core.set_var` writes one of the Board's `vars` and is typed by that + var's declaration rather than by a manifest field. - The three nesting verbs drive reference scope and derived layout; `core.map` drives + The four nesting verbs drive reference scope and derived layout; `core.map` drives reference scope alone; `core.set_var` drives neither and is here because its `value` is a Slot no manifest can type. + Two of them BIND a name for the children they own, and both bind it the same way: as an + output of the container Step itself, read as `{{steps..}}`. A loop's + `item` and a try's `error` therefore cost no namespace root and no bare token — ADR-0014 + closed the roots precisely so a structural idea could not take a word away from users — + and two nested containers cannot shadow each other, because two Steps cannot share an id + on one Board. + `core.map` is the one component whose outputs a manifest cannot declare, because they are whatever the user named. Its `with.entries` is a list of `{key, value, type}`, and a downstream step addresses them as `{{steps..}}` with the declared type — which is @@ -315,7 +323,34 @@ $defs: type: array description: >- Loop children, nested directly with no branch wrapper. On `core.for_each` and - `core.repeat`. + `core.repeat`, and a `core.try`'s protected body. + items: { $ref: "#/$defs/step" } + handler: + type: array + description: >- + A `core.try`'s fallback region, and only meaningful there. The body under `steps:` runs; + if it fails, this runs instead of the rest of it. + + A key beside `steps:` rather than a pair of `branches:` under reserved labels. A Branch's + identity is its `label`, which is free text the user renames — putting "which region is + this" into a string a user edits makes the meaning of the document depend on a display + name, and it costs the schema its first reserved word. A key cannot collide with anything + a user chooses, because nothing inside a step is user-named. + + The failure is exposed to THESE children and to nothing else, as + `{{steps..error}}` — the container's own output, which is how `core.for_each` + already exposes `item`. The body cannot see it, because the body is what produces it; a + Step after the try cannot either, because whether there was a failure at all is a + run-time fact. + + Handler children cannot read the body's Steps. The body failed somewhere, and which of + its Steps completed is not a property of the document — offering them would make scope an + intersection over paths, which is the exact analysis ADR-0013 refuses edges to avoid. It + is the same rule that keeps a Fork's sibling branches out of each other's scope, and it + needs no code of its own: the two regions are siblings. + + Error-type matching needs no matcher: a `core.fork` inside the handler branches on + `{{steps..error.type}}`. items: { $ref: "#/$defs/step" } until: type: string diff --git a/source/sdk/go/definition.go b/source/sdk/go/definition.go index 9f735d1..1761efb 100644 --- a/source/sdk/go/definition.go +++ b/source/sdk/go/definition.go @@ -113,6 +113,19 @@ type Step struct { With map[string]any `yaml:"with,omitempty"` Branches []Branch `yaml:"branches,omitempty"` Steps []Step `yaml:"steps,omitempty"` + // Handler is a core.try's fallback region, and only meaningful there. The + // body under Steps runs; if it fails, this runs instead of the rest of it. + // + // A key beside Steps rather than a pair of Branches under reserved labels: a + // branch's identity is its Label, which is free text a user renames, and + // putting "which region is this" into a display name makes the meaning of the + // document depend on one. Nothing inside a step is user-named, so a key + // collides with nothing. + // + // The failure is exposed to THESE children and to nothing else, as + // `{{steps..error}}` — the container's own output, the way + // core.for_each already exposes `item`. + Handler []Step `yaml:"handler,omitempty"` // Until is a core.repeat's termination condition, tested AFTER the body — // so a repeat always runs its children at least once. A structural key // beside Steps rather than a field under With, for the reason a branch's @@ -206,8 +219,33 @@ const ( ForEachVerb = "core.for_each" RepeatVerb = "core.repeat" SetVarVerb = "core.set_var" + // TryVerb protects a region and falls back to a handler. The one container + // with two child regions. Its retry policy sits in With as ordinary manifest + // fields rather than in a structural key: `until` had to leave With because + // FieldKindTypes has no mappable boolean, and an attempt count is a number, + // which IS a mappable kind — so that argument does not reach here. + TryVerb = "core.try" ) +// ForEachListField is the field a core.for_each iterates, and the one `item` is +// resolved through. A constant rather than a literal at three call sites, +// because it is a name two languages and one manifest have to agree on: a reader +// looking under a different key resolves `item` to nothing. +const ForEachListField = "list" + +// ItemBinding is the output key a loop binds for the children it owns, read as +// `{{steps..item}}`. +// +// An ordinary manifest output of the container step, which is the whole binding +// mechanism and the reason it costs no namespace root: ADR-0014 closed the path +// roots so a structural idea could not take a bare word away from users, and a +// step id already sits one segment below `steps.`. Two nested loops cannot +// shadow each other, because two steps cannot share an id on one Board. +const ItemBinding = "item" + +// ErrorBinding is the output key a core.try binds for its handler's children. +const ErrorBinding = "error" + // VarsOn reports the variables one Board declares: the workflow's at the root, a // block's inside one. // @@ -252,6 +290,7 @@ func CallsOf(steps []Step) []string { walk(branch.Steps) } walk(step.Steps) + walk(step.Handler) } } walk(steps) @@ -346,6 +385,10 @@ func BlockOf(d Definition, id string) *Block { } // WalkSteps visits every step depth-first, parents before children. +// +// Every region a container owns is walked here and nowhere else: a fork's +// branches, a loop body, and a core.try's handler. A region this forgets is a +// region no rule ever sees — the validator reports nothing about it, silently. func WalkSteps(steps []Step, visit func(Step)) { for _, step := range steps { visit(step) @@ -353,5 +396,6 @@ func WalkSteps(steps []Step, visit func(Step)) { WalkSteps(branch.Steps, visit) } WalkSteps(step.Steps, visit) + WalkSteps(step.Handler, visit) } } diff --git a/source/sdk/go/diagnostics.gen.go b/source/sdk/go/diagnostics.gen.go index 61add40..bb331c3 100644 --- a/source/sdk/go/diagnostics.gen.go +++ b/source/sdk/go/diagnostics.gen.go @@ -23,6 +23,9 @@ const ( CodeForkNeedsTwoBranches DefinitionCode = "FORK_NEEDS_TWO_BRANCHES" CodeBranchUnreachableAfter DefinitionCode = "BRANCH_UNREACHABLE_AFTER" CodeLoopHasNoBody DefinitionCode = "LOOP_HAS_NO_BODY" + CodeTryHasNoBody DefinitionCode = "TRY_HAS_NO_BODY" + CodeTryHasNoHandler DefinitionCode = "TRY_HAS_NO_HANDLER" + CodeLoopListNotAList DefinitionCode = "LOOP_LIST_NOT_A_LIST" CodeRepeatHasNoCondition DefinitionCode = "REPEAT_HAS_NO_CONDITION" CodeVarUnknown DefinitionCode = "VAR_UNKNOWN" CodeBlockUnknown DefinitionCode = "BLOCK_UNKNOWN" @@ -53,6 +56,9 @@ var DefinitionDiagnostics = map[DefinitionCode]DefinitionDiagnosticSpec{ CodeForkNeedsTwoBranches: {Code: CodeForkNeedsTwoBranches, Blocks: BlocksPublish, Message: "A fork needs at least two branches — add the other path."}, CodeBranchUnreachableAfter: {Code: CodeBranchUnreachableAfter, Blocks: BlocksPublish, Message: "\"{label}\" has no condition, so nothing after it can ever run. Only the last branch may be unconditional."}, CodeLoopHasNoBody: {Code: CodeLoopHasNoBody, Blocks: BlocksPublish, Message: "This loop repeats nothing. Add at least one Step inside it."}, + CodeTryHasNoBody: {Code: CodeTryHasNoBody, Blocks: BlocksPublish, Message: "This try protects nothing. Add at least one Step to its body."}, + CodeTryHasNoHandler: {Code: CodeTryHasNoHandler, Blocks: BlocksPublish, Message: "This try has no handler, so a failure has nowhere to go. Add a Step to `handler:`."}, + CodeLoopListNotAList: {Code: CodeLoopListNotAList, Blocks: BlocksPublish, Message: "\"{name}\" is {actual}, not a list, so there is nothing to iterate."}, CodeRepeatHasNoCondition: {Code: CodeRepeatHasNoCondition, Blocks: BlocksPublish, Message: "This repeat has no condition, so nothing ever ends it. Give it an `until`."}, CodeVarUnknown: {Code: CodeVarUnknown, Blocks: BlocksPublish, Message: "No variable called \"{name}\" is declared on this board."}, CodeBlockUnknown: {Code: CodeBlockUnknown, Blocks: BlocksPublish, Message: "No block called \"{name}\" is declared in this workflow."}, diff --git a/source/sdk/go/load.go b/source/sdk/go/load.go index f468670..c64c684 100644 --- a/source/sdk/go/load.go +++ b/source/sdk/go/load.go @@ -288,6 +288,11 @@ func validateBranches(steps []Step, prefix string) error { if err := validateBranches(s.Steps, prefix); err != nil { return err } + // A core.try's handler is a step list like any other, and a region this + // forgets is one nothing below it is ever held to. + if err := validateBranches(s.Handler, prefix); err != nil { + return err + } } return nil } diff --git a/source/sdk/go/slots.go b/source/sdk/go/slots.go index 86195be..84ceaed 100644 --- a/source/sdk/go/slots.go +++ b/source/sdk/go/slots.go @@ -1,6 +1,8 @@ package hatua import ( + "strings" + "hatua.dev/go/expressions" ) @@ -257,6 +259,21 @@ func UpstreamOf(doc Definition, ref StepRef) []Step { return []Step{} } +// collectUpstream walks one Board, and treats exactly one verb as more than a +// container. +// +// A core.try has two child regions and they are SIBLINGS, so the body cannot see +// the handler and the handler cannot see the body's steps, with no code saying +// so — the same rule that keeps a fork's branches out of each other's scope. It +// is the right rule for the right reason: the body failed somewhere, and which +// of its steps completed before it did is not a fact the document holds, so +// offering them would make scope an intersection over paths. +// +// What IS special is the try step itself. It appears in the upstream list only +// for steps inside its Handler, because that is where its binding means +// something. Its body cannot read it — the body is what produces it — and +// neither can a step after the try, because whether there was a failure at all +// is decided during a run. func collectUpstream(steps []Step, id string, ancestors []Step) []Step { var earlier []Step @@ -265,16 +282,29 @@ func collectUpstream(steps []Step, id string, ancestors []Step) []Step { return append(append([]Step{}, ancestors...), earlier...) } - seen := append(append(append([]Step{}, ancestors...), earlier...), step) + above := append(append([]Step{}, ancestors...), earlier...) + // The handler is the one region the try itself is in scope for. + inHandler := append(append([]Step{}, above...), step) + outside := inHandler + if step.Use == TryVerb { + outside = above + } + for _, branch := range step.Branches { - if found := collectUpstream(branch.Steps, id, seen); found != nil { + if found := collectUpstream(branch.Steps, id, outside); found != nil { return found } } - if found := collectUpstream(step.Steps, id, seen); found != nil { + if found := collectUpstream(step.Steps, id, outside); found != nil { return found } - earlier = append(earlier, step) + if found := collectUpstream(step.Handler, id, inHandler); found != nil { + return found + } + + if step.Use != TryVerb { + earlier = append(earlier, step) + } } return nil } @@ -393,6 +423,25 @@ func BoardScope(doc Definition, board BoardID, manifests []Manifest, context []C // two are joined here because neither side owns both. Only steps are // constrained by tree position, because only a step can fail to run. func ScopeFor(doc Definition, ref StepRef, manifests []Manifest, context []ContextKey) []expressions.ScopeEntry { + return scopeAt(doc, ref, manifests, context, nil) +} + +// scopeAt is ScopeFor plus the set of loops already being resolved. +// +// `item` is typed by reading the loop's own List field, which means typing an +// expression against the loop step's scope — so building one step's scope can +// ask for another's. The walk terminates on its own, because a step's upstream +// is always strictly earlier in the tree than the step itself and a loop can +// therefore never be its own. resolving is not that argument: it is the guard for +// a document where two steps share an id, which the schema permits into a file +// and StepIdDuplicate reports rather than refuses. +func scopeAt( + doc Definition, + ref StepRef, + manifests []Manifest, + context []ContextKey, + resolving map[string]bool, +) []expressions.ScopeEntry { byUse := make(map[string]Manifest, len(manifests)) for _, manifest := range manifests { byUse[manifest.Use] = manifest @@ -414,13 +463,106 @@ func ScopeFor(doc Definition, ref StepRef, manifests []Manifest, context []Conte manifest := byUse[step.Use] entries = append(entries, expressions.ScopeEntry{ Path: "steps." + step.ID, - Type: stepOutputType(byID, step, manifest), + Type: stepOutputType(doc, ref.Board, byID, step, manifest, manifests, context, resolving), }) } return entries } +// TypeAtPath is the type one path names, read out of a scope, or false when +// nothing declares it. +// +// Longest prefix first, because a scope path is dotted and is one entry rather +// than two: `steps.s2` is an entry and `steps` is not, so `steps.s2.messages` has +// to try three segments before two. The same rule validate.go walks a member +// chain by, restated for a caller that has a path and no expression — reading a +// declared type is not checking one, and reaching for the checker would mean +// manufacturing diagnostics nobody asked for in order to throw them away. +func TypeAtPath(scope []expressions.ScopeEntry, path string) (expressions.TypeNode, bool) { + segments := strings.Split(path, ".") + for take := len(segments); take > 0; take-- { + head := strings.Join(segments[:take], ".") + var node expressions.TypeNode + found := false + for _, entry := range scope { + if entry.Path == head { + node = entry.Type + found = true + break + } + } + if !found { + continue + } + + for _, name := range segments[take:] { + member, held := node.Members[name] + if !held { + return expressions.TypeNode{}, false + } + node = member + } + return node, true + } + return expressions.TypeNode{}, false +} + +// LoopElementType is what one element of a core.for_each's list is, or false when +// the document does not say. +// +// This is the whole of `t: item`. The loop's List is a `ref` field, so its +// declared type is unknown and the ordinary Slot check learns nothing from it; +// the shape is one level below whatever it points at, which is exactly the `of:` +// the source output declared. False when List is missing, is not a plain +// Reference, names nothing, or names something that is not a list — and false +// means `item` stays `item`, which the checker treats as matching anything. +// Guessing `object` instead would be a shape nothing declared. +func LoopElementType( + doc Definition, + board BoardID, + step Step, + manifests []Manifest, + context []ContextKey, +) (expressions.TypeNode, bool) { + return loopElementType(doc, board, step, manifests, context, nil) +} + +func loopElementType( + doc Definition, + board BoardID, + step Step, + manifests []Manifest, + context []ContextKey, + resolving map[string]bool, +) (expressions.TypeNode, bool) { + template, ok := step.With[ForEachListField].(string) + if !ok { + return expressions.TypeNode{}, false + } + path := expressions.SourceReference(template) + if path == "" { + return expressions.TypeNode{}, false + } + + key := StepKey(board, step.ID) + if resolving[key] { + return expressions.TypeNode{}, false + } + deeper := make(map[string]bool, len(resolving)+1) + for held := range resolving { + deeper[held] = true + } + deeper[key] = true + + scope := scopeAt(doc, StepRef{Board: board, ID: step.ID}, manifests, context, deeper) + node, found := TypeAtPath(scope, path) + if !found || node.Type != expressions.TypeList { + return expressions.TypeNode{}, false + } + return expressions.ElementOf(node), true +} + // declarationToType turns a block's parameter or output into the shape the // checker wants. Three lines rather than a second traversal, because a // Declaration is spelled exactly as an Output is. @@ -511,7 +653,16 @@ func variableToType(variable Variable) expressions.TypeNode { // they are whatever the user named. It is the third verb Hatua interprets // structurally, alongside core.fork and core.for_each — and the only one that // does so by reading a field's value rather than its position in the tree. -func stepOutputType(blocks map[string]*Block, step Step, manifest Manifest) expressions.TypeNode { +func stepOutputType( + doc Definition, + board BoardID, + blocks map[string]*Block, + step Step, + manifest Manifest, + manifests []Manifest, + context []ContextKey, + resolving map[string]bool, +) expressions.TypeNode { if called, ok := BlockIDOf(step.Use); ok { return blockOutputType(blocks[called]) } @@ -522,7 +673,33 @@ func stepOutputType(blocks map[string]*Block, step Step, manifest Manifest) expr } return expressions.TypeNode{Type: expressions.TypeObject, Members: members} } - return outputsToType(manifest.Outputs) + + declared := outputsToType(manifest.Outputs) + if step.Use != ForEachVerb { + return declared + } + + // A loop's binding, and the one output whose type is not in the manifest. + // + // Substituted here rather than in outputsToType because it is a property of + // the STEP and not of the declaration: two core.for_each steps share one + // manifest and iterate two different lists. Only a top-level output is + // substituted — `item` nested inside another output's `of:` would be a loop's + // element appearing as a member of something that is not the loop, which no + // manifest can mean. + element, resolved := loopElementType(doc, board, step, manifests, context, resolving) + if !resolved { + return declared + } + members := make(map[string]expressions.TypeNode, len(declared.Members)) + for key, node := range declared.Members { + if node.Type == expressions.TypeItem { + members[key] = element + continue + } + members[key] = node + } + return expressions.TypeNode{Type: expressions.TypeObject, Members: members} } // outputsToType turns a manifest's list of {k, t, of} into the tree the checker diff --git a/source/sdk/go/slots_test.go b/source/sdk/go/slots_test.go index 09b3d87..2d8554d 100644 --- a/source/sdk/go/slots_test.go +++ b/source/sdk/go/slots_test.go @@ -423,3 +423,211 @@ func TestRootBoardIsNotHijackedByAnEmptyBlockID(t *testing.T) { t.Fatalf("the root Board lost its own scope: %v", paths(scope)) } } + +// core.try and the `item` binding, on the side the rules corpus cannot reach. +// +// The corpus compares diagnostics. What a container BINDS is a type, and a type +// reaches a user through scope and the checker — so it is asserted here, and in +// packages/model/src/loops.test.ts assertion for assertion. + +func bindingManifests() []Manifest { + return []Manifest{ + { + Kind: "component", + Use: "component.inbox.fetch", + Name: "Fetch inbox", + Outputs: []Output{ + {K: "messages", Label: "Messages", T: "list", Of: []Output{ + {K: "subject", Label: "Subject", T: "text"}, + }}, + {K: "count", Label: "Count", T: "number"}, + }, + }, + { + Kind: "component", + Use: "core.for_each", + Name: "For each", + Fields: []Field{{K: "list", Label: "List", Kind: "ref", Req: true}}, + // The escape hatch, and the reason this file exists: `item` is not a + // shape the manifest holds. + Outputs: []Output{{K: "item", Label: "Item", T: "item"}}, + }, + { + Kind: "component", + Use: "core.try", + Name: "Try", + Outputs: []Output{ + {K: "error", Label: "Error", T: "object", Of: []Output{ + {K: "message", Label: "Message", T: "text"}, + }}, + }, + }, + {Kind: "component", Use: "component.email.send", Name: "Send"}, + } +} + +func triedDoc() Definition { + return Definition{ + ID: "wf", Name: "W", Version: 1, Status: StatusDraft, + Steps: []Step{ + {ID: "before", Use: "component.email.send"}, + { + ID: "guard", + Use: TryVerb, + Steps: []Step{{ID: "body", Use: "component.email.send"}}, + Handler: []Step{{ID: "rescue", Use: "component.email.send"}}, + }, + {ID: "after", Use: "component.email.send"}, + }, + } +} + +// The handler's children see the try, and therefore the failure they are +// handling. +func TestATryBindsItsFailureToTheHandlerAlone(t *testing.T) { + doc := triedDoc() + + handler := ScopeFor(doc, StepRef{Board: RootBoard, ID: "rescue"}, bindingManifests(), nil) + if !has(handler, "steps.guard") { + t.Fatalf("expected the try in a handler child's scope: %v", paths(handler)) + } + found := expressions.Validate("{{ steps.guard.error.message }}", expressions.TypeText, + expressions.CheckContext{Scope: handler, Functions: expressions.CoreFunctions()}) + if len(found) != 0 { + t.Fatalf("expected the failure to type-check, got %v", found) + } + + // The body PRODUCES the failure, so reading it there would be reading a value + // that cannot exist where it stands. + body := ScopeFor(doc, StepRef{Board: RootBoard, ID: "body"}, bindingManifests(), nil) + if has(body, "steps.guard") { + t.Fatalf("the body saw the failure it produces: %v", paths(body)) + } + + // Past the try, whether there was a failure at all is a run-time fact. + after := ScopeFor(doc, StepRef{Board: RootBoard, ID: "after"}, bindingManifests(), nil) + if has(after, "steps.guard") { + t.Fatalf("a step after the try saw the failure: %v", paths(after)) + } +} + +// The two regions are siblings, which is what a fork's branches already are: +// which of the body's steps completed before the failure is not a property of +// the document. +func TestATrysRegionsCannotSeeEachOther(t *testing.T) { + doc := triedDoc() + + handler := ScopeFor(doc, StepRef{Board: RootBoard, ID: "rescue"}, bindingManifests(), nil) + if has(handler, "steps.body") { + t.Fatalf("a handler child read the body's steps: %v", paths(handler)) + } + body := ScopeFor(doc, StepRef{Board: RootBoard, ID: "body"}, bindingManifests(), nil) + if has(body, "steps.rescue") { + t.Fatalf("a body child read the handler's steps: %v", paths(body)) + } +} + +func loopingDoc(list string) Definition { + return Definition{ + ID: "wf", Name: "W", Version: 1, Status: StatusDraft, + Steps: []Step{ + {ID: "fetch", Use: "component.inbox.fetch"}, + { + ID: "each", + Use: ForEachVerb, + With: map[string]any{ForEachListField: "{{ " + list + " }}"}, + Steps: []Step{{ID: "s1", Use: "component.email.send"}}, + }, + }, + } +} + +func TestItemIsOneElementOfTheListItsFieldNames(t *testing.T) { + doc := loopingDoc("steps.fetch.messages") + + element, ok := LoopElementType(doc, RootBoard, doc.Steps[1], bindingManifests(), nil) + if !ok { + t.Fatalf("expected the loop's element type to resolve") + } + if element.Type != expressions.TypeObject || element.Members["subject"].Type != expressions.TypeText { + t.Fatalf("expected the source output's members, got %#v", element) + } + + scope := ScopeFor(doc, StepRef{Board: RootBoard, ID: "s1"}, bindingManifests(), nil) + if found := expressions.Validate("{{ steps.each.item.subject }}", expressions.TypeText, + expressions.CheckContext{Scope: scope, Functions: expressions.CoreFunctions()}); len(found) != 0 { + t.Fatalf("expected a member of the item to type-check, got %v", found) + } + if found := expressions.Validate("{{ steps.each.item.subject }}", expressions.TypeNumber, + expressions.CheckContext{Scope: scope, Functions: expressions.CoreFunctions()}); len(found) == 0 { + t.Fatalf("expected a text member to be refused where a number is declared") + } +} + +// The whole reason the binding is an output of the container rather than a bare +// token: two loops are two step ids, so nesting needs no shadowing rule. +func TestNestedLoopsEachResolveTheirOwnItem(t *testing.T) { + manifests := append(bindingManifests(), Manifest{ + Kind: "component", + Use: "component.inbox.threads", + Name: "Threads", + Outputs: []Output{{K: "threads", Label: "Threads", T: "list", Of: []Output{ + {K: "entries", Label: "Entries", T: "list", Of: []Output{ + {K: "body", Label: "Body", T: "text"}, + }}, + }}}, + }) + + doc := Definition{ + ID: "wf", Name: "W", Version: 1, Status: StatusDraft, + Steps: []Step{ + {ID: "fetch", Use: "component.inbox.threads"}, + { + ID: "outer", + Use: ForEachVerb, + With: map[string]any{ForEachListField: "{{ steps.fetch.threads }}"}, + Steps: []Step{{ + ID: "inner", + Use: ForEachVerb, + With: map[string]any{ForEachListField: "{{ steps.outer.item.entries }}"}, + Steps: []Step{{ID: "s1", Use: "component.email.send"}}, + }}, + }, + }, + } + + scope := ScopeFor(doc, StepRef{Board: RootBoard, ID: "s1"}, manifests, nil) + context := expressions.CheckContext{Scope: scope, Functions: expressions.CoreFunctions()} + if found := expressions.Validate("{{ steps.inner.item.body }}", expressions.TypeText, context); len(found) != 0 { + t.Fatalf("expected the inner item to resolve through the outer one, got %v", found) + } + // The outer loop's element is still its own shape — an inner `item` hides + // nothing, because the two live under different step ids. + if found := expressions.Validate("{{ steps.outer.item.entries }}", expressions.TypeList, context); len(found) != 0 { + t.Fatalf("the inner loop shadowed the outer one's item: %v", found) + } +} + +// Not ok rather than a guess. `item` then stays `item`, which the checker treats +// as matching anything — the honest answer where `object` would be a shape +// nothing declared, and where the wrongness is reported by CodeLoopListNotAList +// rather than smuggled into a type. +func TestItemIsUnresolvedWhenTheListIsNotOne(t *testing.T) { + for _, list := range []string{ + "steps.fetch.count", // a number, not a list + "json.parse(steps.fetch.count)", // not a plain Reference + "steps.gone.messages", // names nothing + } { + doc := loopingDoc(list) + if _, ok := LoopElementType(doc, RootBoard, doc.Steps[1], bindingManifests(), nil); ok { + t.Fatalf("expected %q to leave item unresolved", list) + } + } + + // And a loop with no list field at all. + doc := loopingDoc("steps.fetch.messages") + doc.Steps[1].With = nil + if _, ok := LoopElementType(doc, RootBoard, doc.Steps[1], bindingManifests(), nil); ok { + t.Fatalf("expected a loop with no list to leave item unresolved") + } +} diff --git a/source/sdk/go/validity.go b/source/sdk/go/validity.go index dbd1fe8..4bf747e 100644 --- a/source/sdk/go/validity.go +++ b/source/sdk/go/validity.go @@ -3,6 +3,8 @@ package hatua import ( "fmt" "strings" + + "hatua.dev/go/expressions" ) // Whether a Workflow Definition is filled in enough to run — the rules that read @@ -89,7 +91,7 @@ func ValidateDefinition(doc Definition, manifests []Manifest) Validity { all := []Diagnostic{} all = append(all, UnknownComponents(doc, byUse)...) all = append(all, MissingRequiredFields(doc, byUse)...) - all = append(all, MalformedContainers(doc)...) + all = append(all, MalformedContainers(doc, manifests)...) all = append(all, BlockRules(doc)...) found := Validity{ @@ -259,6 +261,36 @@ var setVarFields = []struct{ key, label string }{ {"value", "Value"}, } +// listReferenceOf is the path a loop's List names, when it names exactly one +// value and nothing else. +func listReferenceOf(step Step) string { + template, ok := step.With[ForEachListField].(string) + if !ok { + return "" + } + return expressions.SourceReference(template) +} + +// loopListType is the declared type of what a loop's List names. +// +// Deliberately the same read LoopElementType performs, one step short of taking +// the element: the diagnostic and the binding must agree about which lists are +// lists, and two readings of one field are two answers waiting to disagree — a +// loop reported as iterating a number while `item` resolved anyway, or the +// reverse. +func loopListType( + doc Definition, + board BoardID, + step Step, + manifests []Manifest, +) (expressions.TypeNode, bool) { + path := listReferenceOf(step) + if path == "" { + return expressions.TypeNode{}, false + } + return TypeAtPath(ScopeFor(doc, StepRef{Board: board, ID: step.ID}, manifests, nil), path) +} + // UnknownComponents reports a step or a trigger whose verb nothing declares. // // The two roots fail differently, so they are two codes. A `component.*` or @@ -308,7 +340,7 @@ func UnknownComponents(doc Definition, byUse map[string]Manifest) []Diagnostic { // they mean. Read from the tree rather than from a manifest, because a manifest // cannot express them: a fork's branches and a loop's body are positions in the // document, not fields under `with:`. -func MalformedContainers(doc Definition) []Diagnostic { +func MalformedContainers(doc Definition, manifests []Manifest) []Diagnostic { out := []Diagnostic{} WalkDocument(doc, func(ref StepRef, step Step) { @@ -358,6 +390,47 @@ func MalformedContainers(doc Definition) []Diagnostic { out = append(out, raise(CodeRepeatHasNoCondition, subject, nil)) } + // A core.try is two regions, so it is two codes. + // + // Not CodeLoopHasNoBody, which reads "this loop repeats nothing" — the + // right sentence about a for_each and the wrong one about a verb that is + // not a loop. The fork's pair settled this shape already: separate codes + // where the missing half differs, because the sentence that helps differs + // with it. + if step.Use == TryVerb { + if len(step.Steps) == 0 { + out = append(out, raise(CodeTryHasNoBody, subject, nil)) + } + if len(step.Handler) == 0 { + out = append(out, raise(CodeTryHasNoHandler, subject, nil)) + } + } + + // The rule that makes `t: item` observable. + // + // List is a `ref` field and FieldKindTypes maps `ref` to unknown, so the + // ordinary Slot check accepts anything written there — which is how a loop + // pointed at a number type-checks clean while `item` quietly resolves to + // nothing and matches everything downstream. Only a statically-KNOWN + // conflict is reported, matching the lattice everywhere else. + if step.Use == ForEachVerb { + if named, found := loopListType(doc, ref.Board, step, manifests); found && + named.Type != expressions.TypeList && + named.Type != expressions.TypeUnknown && + named.Type != expressions.TypeItem { + name := listReferenceOf(step) + if name == "" { + name = ForEachListField + } + subject.FieldKey = ForEachListField + out = append(out, raise(CodeLoopListNotAList, subject, map[string]string{ + "name": name, + "actual": string(named.Type), + })) + subject.FieldKey = "" + } + } + if step.Use == SetVarVerb { key, named := step.With["key"].(string) // A missing key is CodeFieldRequired's to report. Resolving an @@ -397,6 +470,21 @@ func alwaysReturns(steps []Step) bool { } continue } + // A core.try discharges the obligation only when BOTH regions do. + // + // The body always runs, so on its own it would look like a repeat. But a + // failure part-way through the body is exactly what a try exists to admit, + // and that path leaves the body without finishing it and enters the handler + // instead. So the guarantee is the conjunction: every path out of a try + // goes through the body OR through the handler, and a region that may skip + // its return leaves one of them open. That is the fork's all-branches + // reasoning, asked of two regions where one of them is conditional. + if step.Use == TryVerb { + if alwaysReturns(step.Steps) && alwaysReturns(step.Handler) { + return true + } + continue + } if step.Use != ForkVerb || len(step.Branches) == 0 { continue } @@ -417,8 +505,12 @@ func alwaysReturns(steps []Step) bool { return false } -// stepLists yields every step list in a tree — the root, each branch's, and each -// loop body's. +// stepLists yields every step list in a tree — the root, each branch's, each +// loop body's, and each core.try's handler. +// +// A region missing from here is a region CodeStepAfterReturn never looks at, so +// a step sitting after a core.return inside a handler would be reported by +// neither language and published by both. func stepLists(steps []Step, visit func([]Step)) { visit(steps) for _, step := range steps { @@ -426,6 +518,7 @@ func stepLists(steps []Step, visit func([]Step)) { stepLists(branch.Steps, visit) } stepLists(step.Steps, visit) + stepLists(step.Handler, visit) } } From 30502a098e53bca4db135ea3147fea2c1954fd97 Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Mon, 24 Aug 2026 16:23:29 +0100 Subject: [PATCH 2/2] fix(model,sdk/go): a list with no of: says nothing, and one output key has one type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two places where the two languages could hand a Host a different answer about the same document. loopElementType answered `object` for a list that declared no `of:`. That is ElementOf's right answer for a projection — what does `.name` read off this — and the wrong one here: it marks item as a shape nothing declared, so {{ steps.each.item }} in a text field raises EXPR_TYPE_MISMATCH, an error that refuses Publish on a correct workflow. It now returns nothing, so item stays item and is accepted with EXPR_TYPE_UNKNOWN — a warning, and the code whose own summary names item members as its motivating case. The function's docstring already said this ("Guessing `object` instead would be a shape nothing declared"); the code did not. blockOutputType was last-wins in Go and first-wins in TypeScript. A block declaring one output key twice is a document the schema loads and DECLARATION_KEY_DUPLICATE only stops at Publish, so the same call site typed number in one builder and text in the other. Go is first-wins now, matching BlockOf and CyclicBlocks. Neither is reachable from the conformance corpus: the expression corpus supplies a scope rather than building one from a document, and the rules corpus never runs the expression pass. So both are pinned by a test in each language instead, and each one fails without its fix. --- source/packages/model/src/blocks.test.ts | 27 ++++++++++++- source/packages/model/src/loops.test.ts | 22 +++++++++++ source/packages/model/src/scope.ts | 8 ++++ source/sdk/go/slots.go | 17 +++++++++ source/sdk/go/slots_test.go | 48 ++++++++++++++++++++++++ 5 files changed, 121 insertions(+), 1 deletion(-) diff --git a/source/packages/model/src/blocks.test.ts b/source/packages/model/src/blocks.test.ts index b19ae9d..0697019 100644 --- a/source/packages/model/src/blocks.test.ts +++ b/source/packages/model/src/blocks.test.ts @@ -3,7 +3,7 @@ import type { Manifest, WorkflowDefinition } from '@hatua/schema' import { describe, expect, it } from 'vitest' import { callSlots, cyclicBlocks, returnSlots } from './blocks' import { indexManifests } from './connections' -import { boardScope, scopeFor } from './scope' +import { blockOutputType, boardScope, scopeFor } from './scope' import { boards, stepKey, walkDocument } from './tree' import { validateDefinition } from './validity' @@ -596,3 +596,28 @@ describe('the contract', () => { expect(validate('{{ var.attempt_note }}', 'text', { scope, functions: new Map() })).toEqual([]) }) }) + +/** + * A repeated output key, which the schema permits into a file and + * DECLARATION_KEY_DUPLICATE only stops at Publish — so both languages have to + * pick the same one of the two while the document is being edited. + * + * First-wins, matching `blockOf` and `cyclicBlocks`. Which one is picked matters + * less than that one answer exists: last here and first in the Go SDK types the + * same call site `number` in one builder and `text` in the other, and a + * hand-edited document then checks differently depending on who opened it. + */ +describe('a block that declares one output twice', () => { + it('types the call site from the first declaration', () => { + const block = { + id: 'twice', + outputs: [ + { k: 'out', label: 'Out', t: 'text' as const }, + { k: 'out', label: 'Out', t: 'number' as const }, + ], + steps: [], + } + + expect(blockOutputType(block)).toEqual({ type: 'object', members: { out: { type: 'text' } } }) + }) +}) diff --git a/source/packages/model/src/loops.test.ts b/source/packages/model/src/loops.test.ts index 96c4852..75bdce4 100644 --- a/source/packages/model/src/loops.test.ts +++ b/source/packages/model/src/loops.test.ts @@ -204,6 +204,7 @@ const listing = (): Manifest[] => [ of: [{ k: 'subject', label: 'Subject', t: 'text' }], }, { k: 'count', label: 'Count', t: 'number' }, + { k: 'tags', label: 'Tags', t: 'list' }, ], }, { @@ -395,6 +396,27 @@ describe('what a core.for_each binds', () => { expect(loopElementType(document, null, document.steps[1] as Step, listing())).toBeNull() }) + /** + * A list with no `of:` is a list whose elements the document says nothing + * about, which is not the same as a list of objects with no members. `item` + * stays `item` and matches anything, so writing one into a `text` field is + * accepted and checked at run time — EXPR_TYPE_UNKNOWN, a warning, which is + * the code whose own summary names `item` members as its motivating case. + * + * Answering `object` here marks `item` as a shape nothing declared, and then + * every scalar field it is written into reports EXPR_TYPE_MISMATCH — an error + * that refuses Publish on a document that is correct. + */ + it('is nothing when the list declared no `of:`, so `item` still matches anything', () => { + const document = looping('steps.fetch.tags') + expect(loopElementType(document, null, document.steps[1] as Step, listing())).toBeNull() + + const scope = scopeFor(document, { board: null, id: 's1' }, listing()) + const found = validate('{{ steps.each.item }}', 'text', { scope, functions: coreFunctions() }) + expect(found.map((one) => one.code)).toEqual(['EXPR_TYPE_UNKNOWN']) + expect(found.map((one) => one.severity)).toEqual(['warning']) + }) + it('is nothing when the list is not a plain Reference, or names nothing at all', () => { const computed = looping('json.parse(steps.fetch.count)') expect(loopElementType(computed, null, computed.steps[1] as Step, listing())).toBeNull() diff --git a/source/packages/model/src/scope.ts b/source/packages/model/src/scope.ts index 8a11fd4..bd0ea3c 100644 --- a/source/packages/model/src/scope.ts +++ b/source/packages/model/src/scope.ts @@ -365,6 +365,14 @@ export function loopElementType( const node = typeAtPath(scope, path) if (!node || node.type !== 'list') return null + // A list that declared no `of:` says nothing about its elements, so there is + // no shape to hand back. `elementOf` answers `object` for one — which is the + // right answer for a projection, where the question is "what does `.name` + // read off this?", and the wrong one here: it would mark `item` as an object + // nothing declared, and every scalar field it is written into would report a + // mismatch against a shape the document never said. + if (!node.members) return null + return elementOf(node) } diff --git a/source/sdk/go/slots.go b/source/sdk/go/slots.go index 84ceaed..66649cd 100644 --- a/source/sdk/go/slots.go +++ b/source/sdk/go/slots.go @@ -560,6 +560,15 @@ func loopElementType( if !found || node.Type != expressions.TypeList { return expressions.TypeNode{}, false } + // A list that declared no `of:` says nothing about its elements, so there is + // no shape to hand back. ElementOf answers object for one — the right answer + // for a projection, where the question is what `.name` reads off this, and + // the wrong one here: it would mark item as an object nothing declared, and + // every scalar field it is written into would report a mismatch against a + // shape the document never said. + if len(node.Members) == 0 { + return expressions.TypeNode{}, false + } return expressions.ElementOf(node), true } @@ -590,6 +599,14 @@ func blockOutputType(block *Block) expressions.TypeNode { return node } for _, output := range block.Outputs { + // First-wins, matching the TypeScript half. A repeated output key is a + // document a hand-edit reaches and DECLARATION_KEY_DUPLICATE only stops + // Publish, so both languages have to pick the same one of the two — last + // here and first there types the same call site number in one builder + // and text in the other. + if _, taken := node.Members[output.K]; taken { + continue + } node.Members[output.K] = declarationToType(output) } return node diff --git a/source/sdk/go/slots_test.go b/source/sdk/go/slots_test.go index 2d8554d..41261d4 100644 --- a/source/sdk/go/slots_test.go +++ b/source/sdk/go/slots_test.go @@ -441,6 +441,7 @@ func bindingManifests() []Manifest { {K: "subject", Label: "Subject", T: "text"}, }}, {K: "count", Label: "Count", T: "number"}, + {K: "tags", Label: "Tags", T: "list"}, }, }, { @@ -631,3 +632,50 @@ func TestItemIsUnresolvedWhenTheListIsNotOne(t *testing.T) { t.Fatalf("expected a loop with no list to leave item unresolved") } } + +// A list with no `of:` is a list whose elements the document says nothing about, +// which is not the same as a list of objects with no members. `item` stays +// `item` and matches anything, so writing one into a text field is accepted and +// checked at run time — EXPR_TYPE_UNKNOWN, a warning. +// +// Answering object here marks `item` as a shape nothing declared, and then every +// scalar field it is written into reports EXPR_TYPE_MISMATCH: an error that +// refuses Publish on a document that is correct. The TypeScript half asserts the +// same thing in `packages/model/src/loops.test.ts`, because a builder and a +// runner disagreeing about `item` is the whole reason this file exists. +func TestLoopElementTypeIsUnresolvedWhenTheListDeclaredNoOf(t *testing.T) { + doc := loopingDoc("steps.fetch.tags") + + if _, ok := LoopElementType(doc, RootBoard, doc.Steps[1], bindingManifests(), nil); ok { + t.Fatalf("expected a list with no `of:` to leave item unresolved") + } + + scope := ScopeFor(doc, StepRef{Board: RootBoard, ID: "s1"}, bindingManifests(), nil) + found := expressions.Validate("{{ steps.each.item }}", expressions.TypeText, + expressions.CheckContext{Scope: scope, Functions: expressions.CoreFunctions()}) + if len(found) != 1 || found[0].Code != "EXPR_TYPE_UNKNOWN" { + t.Fatalf("expected item to be accepted and checked at run time, got %v", found) + } +} + +// A repeated output key, which the schema permits into a file and +// DECLARATION_KEY_DUPLICATE only stops at Publish — so both languages have to +// pick the same one of the two while the document is being edited. +// +// First-wins, matching BlockOf and CyclicBlocks and the TypeScript half. Which +// one is picked matters less than that one answer exists: last here and first +// there types the same call site number in one builder and text in the other. +func TestBlockOutputTypeTakesTheFirstOfARepeatedKey(t *testing.T) { + block := Block{ + ID: "twice", + Outputs: []Declaration{ + {K: "out", Label: "Out", T: "text"}, + {K: "out", Label: "Out", T: "number"}, + }, + } + + node := blockOutputType(&block) + if node.Members["out"].Type != expressions.TypeText { + t.Fatalf("expected the first declaration to win, got %#v", node.Members["out"]) + } +}