From 9b44a85bd149e6190849c87cb7584f8750c0ca23 Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Sun, 23 Aug 2026 08:13:05 +0100 Subject: [PATCH 01/10] feat(schema): core.repeat and core.set_var, and a variable that declares its type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The schema documented `core.set_var` as how a var is written and nothing implemented it; `core.repeat` appeared in no code at all. Both are declared here, together with the corpus scenarios both languages are then held to. Three shapes are settled. `until:` is a structural key beside `steps:`, not a field under `with:`. A manifest field carries a rendering kind and no type, so FIELD_KIND_TYPES is what recovers the expected type — and that vocabulary has no mappable boolean at all. Under `with:` a repeat's condition would type-check as text, which is the half of the contract the field exists to carry. A Branch's `when` sits in the same position for the same reason. `core.set_var` takes a var key and a value typed by the var that key names. No manifest knows which Board a step is on, so it joins a call and a `core.return` as a verb whose fields are read from the document rather than from a manifest. A variable declares `t`, and `of` for shape. `value` is only the FIRST value once a step can write the same variable, so a type read off it is a claim about one moment in an execution — the builder says text while the runner produces a number, and every downstream check was answered against that. Required rather than defaulted, because a fallback spelling is a second definition of the thing on the day it was declared. The corpus lands before either implementation. Its load-bearing pair sits side by side: a repeat discharges a block's return obligation because its body always runs, and a for_each does not because the list may be empty. --- .../rules/repeat-and-variables.yaml | 326 ++++++++++++++++++ .../model/src/generated/diagnostics.ts | 12 + .../schema/src/generated/definition.ts | 28 +- source/schemas/definition-diagnostics.yaml | 44 ++- .../schemas/workflow-definition.schema.yaml | 71 +++- source/sdk/go/diagnostics.gen.go | 4 + 6 files changed, 467 insertions(+), 18 deletions(-) create mode 100644 source/conformance/definition/rules/repeat-and-variables.yaml diff --git a/source/conformance/definition/rules/repeat-and-variables.yaml b/source/conformance/definition/rules/repeat-and-variables.yaml new file mode 100644 index 0000000..b67ca3e --- /dev/null +++ b/source/conformance/definition/rules/repeat-and-variables.yaml @@ -0,0 +1,326 @@ +about: >- + `core.repeat` and `core.set_var`, as both languages must report them. + + Two verbs Hatua interprets structurally and a manifest cannot describe. A + repeat's `until` is a key beside `steps:`, not a field under `with:`, because + `FIELD_KIND_TYPES` has no mappable boolean. A `core.set_var`'s `key` names one + of its own Board's `vars` and its `value` is typed by that var's declaration, + so neither field is a manifest's to declare either. + + The load-bearing pair is here: a repeat DISCHARGES a block's return obligation + and a `core.for_each` does not, because a repeat tests its condition after the + body and therefore always runs it once, while a list may be empty. Both + scenarios sit side by side so a language that collapses the two fails on one of + them. + +manifests: + - kind: component + use: component.email.send + name: Send email + fields: + - { k: to, label: To, kind: text, req: true } + outputs: [] + - kind: component + use: core.fork + name: Branch + fields: [] + outputs: [] + - kind: component + use: core.for_each + name: For each + fields: [] + outputs: [] + - kind: component + use: core.repeat + name: Repeat + fields: [] + outputs: [] + - kind: component + use: core.set_var + name: Set a variable + fields: [] + outputs: [] + - kind: component + use: core.return + name: Return + fields: [] + outputs: [] + - kind: trigger + use: core.schedule + name: Schedule + fields: [] + outputs: [] + +scenarios: + # ---- core.repeat ---------------------------------------------------------- + + - name: a repeat with a condition and a body reports nothing + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - id: again + use: core.repeat + until: "{{ var.done }}" + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + vars: + - { key: done, t: boolean, value: false } + expect: [] + + - name: a repeat with no condition never ends, and says so + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - id: again + use: core.repeat + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + expect: + - { code: REPEAT_HAS_NO_CONDITION, blocks: publish, stepId: again } + + - name: a condition of nothing but whitespace is no condition + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - id: again + use: core.repeat + until: " " + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + expect: + - { code: REPEAT_HAS_NO_CONDITION, blocks: publish, stepId: again } + + - name: an empty repeat is both an empty body and a missing condition + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - { id: again, use: core.repeat } + expect: + - { code: LOOP_HAS_NO_BODY, blocks: publish, stepId: again } + - { code: REPEAT_HAS_NO_CONDITION, blocks: publish, stepId: again } + + - name: an empty body is one code whichever loop verb it is + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - { id: each, use: core.for_each } + - { id: again, use: core.repeat, until: "{{ var.done }}", steps: [] } + vars: + - { key: done, t: boolean, value: false } + expect: + - { code: LOOP_HAS_NO_BODY, blocks: publish, stepId: each } + - { code: LOOP_HAS_NO_BODY, blocks: publish, stepId: again } + + - name: a repeat's condition is not held to a manifest, so a repeat needs no fields + definition: + id: wf + name: W + version: 1 + status: draft + steps: + - id: again + use: core.repeat + until: "{{ var.done }}" + with: {} + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + vars: + - { key: done, t: boolean, value: false } + expect: [] + + # ---- a repeat and the return obligation ----------------------------------- + + - name: a repeat whose body always returns discharges the obligation, because the body always runs + definition: + id: wf + name: W + version: 1 + status: draft + steps: [] + blocks: + - id: ask + outputs: [{ k: answer, label: Answer, t: text }] + steps: + - id: again + use: core.repeat + until: "{{ var.done }}" + steps: + - { id: ret, use: core.return, with: { answer: "yes" } } + vars: + - { key: done, t: boolean, value: false } + expect: [] + + - name: a for_each whose body always returns does not, because the list may be empty + definition: + id: wf + name: W + version: 1 + status: draft + steps: [] + blocks: + - id: ask + outputs: [{ k: answer, label: Answer, t: text }] + steps: + - id: each + use: core.for_each + steps: + - { id: ret, use: core.return, with: { answer: "yes" } } + expect: + - { code: BLOCK_PATH_WITHOUT_RETURN, blocks: publish, blockId: ask } + + - name: a repeat whose body only sometimes returns discharges nothing + definition: + id: wf + name: W + version: 1 + status: draft + steps: [] + blocks: + - id: ask + outputs: [{ k: answer, label: Answer, t: text }] + steps: + - id: again + use: core.repeat + until: "{{ var.done }}" + steps: + - { id: s1, use: component.email.send, with: { to: "me@dane.dev" } } + vars: + - { key: done, t: boolean, value: false } + expect: + - { code: BLOCK_PATH_WITHOUT_RETURN, blocks: publish, blockId: ask } + + - name: nothing after a repeat that always returns can run + definition: + id: wf + name: W + version: 1 + status: draft + steps: [] + blocks: + - id: ask + outputs: [{ k: answer, label: Answer, t: text }] + steps: + - id: again + use: core.repeat + until: "{{ var.done }}" + steps: + - { id: ret, use: core.return, with: { answer: "yes" } } + - { id: after, use: component.email.send, with: { to: "me@dane.dev" } } + vars: + - { key: done, t: boolean, value: false } + expect: + - { code: STEP_AFTER_RETURN, blocks: publish, stepId: after, blockId: ask } + + # ---- core.set_var --------------------------------------------------------- + + - name: a set_var at the root writes the workflow's variable + definition: + id: wf + name: W + version: 1 + status: draft + vars: + - { key: attempt, t: number, value: 0 } + steps: + - { id: bump, use: core.set_var, with: { key: attempt, value: "{{ var.attempt }}" } } + expect: [] + + - name: a set_var naming a variable nothing declares blocks publish + definition: + id: wf + name: W + version: 1 + status: draft + vars: + - { key: attempt, t: number, value: 0 } + steps: + - { id: bump, use: core.set_var, with: { key: attemp, value: "1" } } + expect: + - { code: VAR_UNKNOWN, blocks: publish, stepId: bump, fieldKey: key } + + - name: a set_var inside a block writes that block's variable + definition: + id: wf + name: W + version: 1 + status: draft + steps: [] + blocks: + - id: ask + vars: + - { key: note, t: text, value: "" } + steps: + - { id: bump, use: core.set_var, with: { key: note, value: "again" } } + expect: [] + + - name: a set_var inside a block cannot reach the workflow's variables + definition: + id: wf + name: W + version: 1 + status: draft + vars: + - { key: attempt, t: number, value: 0 } + steps: [] + blocks: + - id: ask + vars: + - { key: note, t: text, value: "" } + steps: + - { id: bump, use: core.set_var, with: { key: attempt, value: "1" } } + expect: + - { code: VAR_UNKNOWN, blocks: publish, stepId: bump, blockId: ask, fieldKey: key } + + - name: a set_var with nothing to write and nowhere to write it reports both + definition: + id: wf + name: W + version: 1 + status: draft + vars: + - { key: attempt, t: number, value: 0 } + steps: + - { id: bump, use: core.set_var, with: {} } + expect: + - { code: FIELD_REQUIRED, blocks: publish, stepId: bump, fieldKey: key } + - { code: FIELD_REQUIRED, blocks: publish, stepId: bump, fieldKey: value } + + - name: a set_var whose value is false has a value, because false is an answer + definition: + id: wf + name: W + version: 1 + status: draft + vars: + - { key: done, t: boolean, value: false } + steps: + - { id: finish, use: core.set_var, with: { key: done, value: false } } + expect: [] + + - name: an unnamed variable is not resolved against the board, so only the missing key is reported + definition: + id: wf + name: W + version: 1 + status: draft + vars: + - { key: attempt, t: number, value: 0 } + steps: + - { id: bump, use: core.set_var, with: { value: "1" } } + expect: + - { code: FIELD_REQUIRED, blocks: publish, stepId: bump, fieldKey: key } diff --git a/source/packages/model/src/generated/diagnostics.ts b/source/packages/model/src/generated/diagnostics.ts index 8facf0b..101e20f 100644 --- a/source/packages/model/src/generated/diagnostics.ts +++ b/source/packages/model/src/generated/diagnostics.ts @@ -14,6 +14,8 @@ export type DefinitionCode = | 'FORK_NEEDS_TWO_BRANCHES' | 'BRANCH_UNREACHABLE_AFTER' | 'LOOP_HAS_NO_BODY' + | 'REPEAT_HAS_NO_CONDITION' + | 'VAR_UNKNOWN' | 'BLOCK_UNKNOWN' | 'BLOCK_RECURSION' | 'RETURN_OUTSIDE_BLOCK' @@ -63,6 +65,16 @@ export const DEFINITION_DIAGNOSTICS: Record /** - * A list of key/value objects rather than a map, so a `type` or `label` can be added later without a breaking change to every existing file. + * A list of key/value objects rather than a map, which is what let `t` be added here without inventing a second spelling for a variable. + * NOT a `declaration`. A declaration is a contract with nothing in it; a variable carries a value, and its key is its own label — the builder shows `var.digest_to`, not a friendly name. `t` and `of` are spelled identically to a declaration's so one function reads both, but three shared fields out of five is not one idea. */ export const variable = z.strictObject({ get key() { return identifier }, /** - * A literal, or an expression evaluated by the SDK's shared evaluator. + * The type every `{{var.}}` is checked against, and the type `core.set_var` must write. Declared rather than inferred from `value`, because `value` is only the FIRST value: a `core.set_var` writing a number into a var that started as `""` would make an inferred `text` a lie, and every downstream type check was answered against it. Required rather than defaulted, for the reason ADR-0014 rewrote every document at once — a fallback spelling is a second definition of the thing on the day it was declared. + * `item` is absent, for the reason it is absent from a declaration: it is the for-each escape hatch, resolved by following a loop's `list` back to its source output, and a variable is not the output of anything. + */ + t: z.enum(['text', 'number', 'boolean', 'datetime', 'object', 'list']), + /** + * Shape of each list element or object member, spelled as a declaration's `of` is. + */ + get of() { + return z.array(declaration).optional() + }, + /** + * The initial value: a literal, or an expression evaluated by the SDK's shared evaluator. Checked against `t` like any other Slot when it is a Template. */ value: z.unknown(), }) @@ -136,7 +148,8 @@ 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 three structurally: `core.fork` creates branches, `core.for_each` nests and exposes `item`, and `core.map` derives its outputs from its own `entries` field rather than from its manifest. The first two drive reference scope and derived layout; the third drives reference scope alone. + * 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. * `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), @@ -151,11 +164,17 @@ export const step = z.strictObject({ return z.array(branch).min(1).optional() }, /** - * Loop children, nested directly with no branch wrapper. Only on `core.for_each`. + * Loop children, nested directly with no branch wrapper. On `core.for_each` and `core.repeat`. */ get steps() { 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. + * Nothing bounds the iterations, and that is deliberate rather than missing: whether an `until` ever goes false depends on run-time values, so unlike recursion it is not a property the document has. Hatua does not execute, so a bound written here would be a number no reader could enforce. Bounding is the Host runner's obligation — see ADR-0013. + */ + until: z.string().optional(), }) export type Step = z.infer @@ -215,6 +234,7 @@ export const workflowDefinition = z.strictObject({ }, /** * Mutable workflow-scoped state, readable as `{{var.}}` and written by `core.set_var`. Distinct from trigger payloads, which arrive from outside. A value may be a literal or an expression, so a var can normalise differently-shaped trigger payloads into one shape. + * Its type is declared, never read off that value — a `core.set_var` writing it later means the value in the file is an initial value rather than a contract. See ADR-0013. */ get vars() { return z.array(variable).optional() diff --git a/source/schemas/definition-diagnostics.yaml b/source/schemas/definition-diagnostics.yaml index 6662faa..a9deb7d 100644 --- a/source/schemas/definition-diagnostics.yaml +++ b/source/schemas/definition-diagnostics.yaml @@ -67,7 +67,37 @@ codes: - code: LOOP_HAS_NO_BODY blocks: publish message: "This loop repeats nothing. Add at least one Step inside it." - summary: A `core.for_each` with no children. + summary: >- + A `core.for_each` or a `core.repeat` with no children. One code rather than + two: the mistake is the same and so is the fix, and the message names + neither verb. + + - code: REPEAT_HAS_NO_CONDITION + blocks: publish + message: "This repeat has no condition, so nothing ever ends it. Give it an `until`." + summary: >- + A `core.repeat` whose `until` is absent or blank. Structural rather than a + required field, because the condition is a structural key beside `steps:` + and not something a manifest declares — the same position a Branch's + `when` holds, and for the same reason: `FIELD_KIND_TYPES` cannot express + "a Template that must produce a boolean". + + Publish-blocking rather than edit-blocking, because a repeat a second after + it is added is exactly this. + + - code: VAR_UNKNOWN + blocks: publish + message: 'No variable called "{name}" is declared on this board.' + summary: >- + A `core.set_var` naming a var its Board does not declare. The BLOCK_UNKNOWN + argument one level down: a var key goes stale the moment someone renames or + removes it, which is ordinary building, so this blocks Publish and never + editing. + + "on this board" is the load-bearing half of the message. A `core.set_var` + inside a block writes that block's vars and can never reach the workflow's, + so naming a workflow var from inside a block is this diagnostic and not a + scoping accident the runner discovers. # ---- blocks --------------------------------------------------------------- @@ -105,10 +135,14 @@ codes: A path returns via a `core.return` at the Board's root level, or a root-level fork that is exhaustive — an unconditional last branch — and whose every branch returns. A fork whose every branch carries a `when` is - not exhaustive: first-match-wins means it can match none of them. A return inside a loop exits - the block early and is legal, but never discharges the obligation: the list - may be empty and the body may never run — the same reasoning that keeps - sibling branches out of scope, applied to time instead of to paths. + not exhaustive: first-match-wins means it can match none of them. + + A `core.repeat` DOES discharge it, and a `core.for_each` never does. That is + one rule rather than two special cases: a repeat tests its `until` after the + body, so the body always runs, while a for_each's list may be empty. The + question is only ever "is this region guaranteed to run at all" — the same + reasoning that keeps sibling branches out of scope, applied to time instead + of to paths. - code: STEP_AFTER_RETURN blocks: publish diff --git a/source/schemas/workflow-definition.schema.yaml b/source/schemas/workflow-definition.schema.yaml index e327768..78c4a3d 100644 --- a/source/schemas/workflow-definition.schema.yaml +++ b/source/schemas/workflow-definition.schema.yaml @@ -69,6 +69,9 @@ properties: Mutable workflow-scoped state, readable as `{{var.}}` and written by `core.set_var`. Distinct from trigger payloads, which arrive from outside. A value may be a literal or an expression, so a var can normalise differently-shaped trigger payloads into one shape. + + Its type is declared, never read off that value — a `core.set_var` writing it later means + the value in the file is an initial value rather than a contract. See ADR-0013. items: { $ref: "#/$defs/variable" } blocks: @@ -228,15 +231,39 @@ $defs: variable: type: object additionalProperties: false - required: [key, value] + required: [key, t, value] description: >- - A list of key/value objects rather than a map, so a `type` or `label` can be added later - without a breaking change to every existing file. + A list of key/value objects rather than a map, which is what let `t` be added here without + inventing a second spelling for a variable. + + NOT a `declaration`. A declaration is a contract with nothing in it; a variable carries a + value, and its key is its own label — the builder shows `var.digest_to`, not a friendly + name. `t` and `of` are spelled identically to a declaration's so one function reads both, + but three shared fields out of five is not one idea. properties: key: $ref: "#/$defs/identifier" + t: + enum: [text, number, boolean, datetime, object, list] + description: >- + The type every `{{var.}}` is checked against, and the type `core.set_var` must + write. Declared rather than inferred from `value`, because `value` is only the FIRST + value: a `core.set_var` writing a number into a var that started as `""` would make an + inferred `text` a lie, and every downstream type check was answered against it. Required + rather than defaulted, for the reason ADR-0014 rewrote every document at once — a + fallback spelling is a second definition of the thing on the day it was declared. + + `item` is absent, for the reason it is absent from a declaration: it is the for-each + escape hatch, resolved by following a loop's `list` back to its source output, and a + variable is not the output of anything. + of: + type: array + description: Shape of each list element or object member, spelled as a declaration's `of` is. + items: { $ref: "#/$defs/declaration" } value: - description: A literal, or an expression evaluated by the SDK's shared evaluator. + description: >- + The initial value: a literal, or an expression evaluated by the SDK's shared evaluator. + Checked against `t` like any other Slot when it is a Template. step: type: object @@ -261,10 +288,15 @@ $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 three structurally: `core.fork` creates - branches, `core.for_each` nests and exposes `item`, and `core.map` derives its outputs - from its own `entries` field rather than from its manifest. The first two drive reference - scope and derived layout; the third drives reference scope alone. + 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. `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 @@ -281,8 +313,29 @@ $defs: items: { $ref: "#/$defs/branch" } steps: type: array - description: Loop children, nested directly with no branch wrapper. Only on `core.for_each`. + description: >- + Loop children, nested directly with no branch wrapper. On `core.for_each` and + `core.repeat`. items: { $ref: "#/$defs/step" } + until: + type: string + description: >- + 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. + + Nothing bounds the iterations, and that is deliberate rather than missing: whether an + `until` ever goes false depends on run-time values, so unlike recursion it is not a + property the document has. Hatua does not execute, so a bound written here would be a + number no reader could enforce. Bounding is the Host runner's obligation — see ADR-0013. branch: type: object diff --git a/source/sdk/go/diagnostics.gen.go b/source/sdk/go/diagnostics.gen.go index ae0fab7..61add40 100644 --- a/source/sdk/go/diagnostics.gen.go +++ b/source/sdk/go/diagnostics.gen.go @@ -23,6 +23,8 @@ const ( CodeForkNeedsTwoBranches DefinitionCode = "FORK_NEEDS_TWO_BRANCHES" CodeBranchUnreachableAfter DefinitionCode = "BRANCH_UNREACHABLE_AFTER" CodeLoopHasNoBody DefinitionCode = "LOOP_HAS_NO_BODY" + CodeRepeatHasNoCondition DefinitionCode = "REPEAT_HAS_NO_CONDITION" + CodeVarUnknown DefinitionCode = "VAR_UNKNOWN" CodeBlockUnknown DefinitionCode = "BLOCK_UNKNOWN" CodeBlockRecursion DefinitionCode = "BLOCK_RECURSION" CodeReturnOutsideBlock DefinitionCode = "RETURN_OUTSIDE_BLOCK" @@ -51,6 +53,8 @@ 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."}, + 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."}, CodeBlockRecursion: {Code: CodeBlockRecursion, Blocks: BlocksPublish, Message: "\"{name}\" calls itself, directly or through another block."}, CodeReturnOutsideBlock: {Code: CodeReturnOutsideBlock, Blocks: BlocksPublish, Message: "`core.return` publishes a block's outputs, and this is not inside a block."}, From e6ebd653e922bbd4263fb3a4f15ba19cd3d706d0 Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Sun, 23 Aug 2026 08:13:20 +0100 Subject: [PATCH 02/10] feat(model,sdk/go): the two verbs, in both languages at once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit validity.ts and validity.go move rule for rule, because the corpus compares them as a sorted set and a code that blocked Publish in one and informed in the other would let a workflow publish from one builder and not another. `alwaysReturns` gains one line rather than a special case: a `core.repeat` discharges the obligation its body discharges. The question it asks is only ever whether a region is guaranteed to run at all — a list may be empty, a repeat's first pass cannot be skipped — so both loop verbs now have one answer instead of two exceptions. `varsOn` is what makes "a set_var inside a block can never reach the workflow's variables" true by construction. There is no second list to fall back to, so naming a workflow var from inside a block is an unknown name rather than a scope the two runtimes might resolve differently. varType is gone from both languages. It also settles a divergence it had no answer for: yaml.v3 decodes `value: 2024-01-01T00:00:00Z` into a time.Time while the builder's parser leaves it a string, so the two typed one scalar differently and the Go SDK carried a comment saying so. loops.test.ts and loops_test.go mirror each other over the half the corpus cannot reach — what a Slot expects and what the checker then says about it, which is where the type marking lives. --- source/packages/model/src/blocks.test.ts | 4 +- source/packages/model/src/fixtures.ts | 2 +- source/packages/model/src/loops.test.ts | 179 +++++++++++++++++++++++ source/packages/model/src/scope.ts | 29 ++-- source/packages/model/src/slots.ts | 89 ++++++++++- source/packages/model/src/validity.ts | 94 ++++++++++-- source/sdk/go/definition.go | 68 ++++++++- source/sdk/go/loops_test.go | 173 ++++++++++++++++++++++ source/sdk/go/slots.go | 129 +++++++++++----- source/sdk/go/validity.go | 65 +++++++- source/sdk/js/src/expression.test.ts | 2 +- source/sdk/js/src/index.ts | 4 + 12 files changed, 765 insertions(+), 73 deletions(-) create mode 100644 source/packages/model/src/loops.test.ts create mode 100644 source/sdk/go/loops_test.go diff --git a/source/packages/model/src/blocks.test.ts b/source/packages/model/src/blocks.test.ts index 6f9718a..b19ae9d 100644 --- a/source/packages/model/src/blocks.test.ts +++ b/source/packages/model/src/blocks.test.ts @@ -45,7 +45,7 @@ const doc = (overrides: Partial = {}): WorkflowDefinition => version: 1, status: 'draft', triggers: [{ id: 'nightly', use: 'core.schedule' }], - vars: [{ key: 'digest_to', value: 'me@dane.dev' }], + vars: [{ key: 'digest_to', t: 'text', value: 'me@dane.dev' }], steps: [], ...overrides, }) @@ -65,7 +65,7 @@ const ARCHIVE: WorkflowDefinition = doc({ }, ], outputs: [{ k: 'url', label: 'Archive URL', t: 'text' }], - vars: [{ key: 'attempt_note', value: '' }], + vars: [{ key: 'attempt_note', t: 'text', value: '' }], steps: [ { id: 'put', diff --git a/source/packages/model/src/fixtures.ts b/source/packages/model/src/fixtures.ts index daf8369..774829b 100644 --- a/source/packages/model/src/fixtures.ts +++ b/source/packages/model/src/fixtures.ts @@ -20,7 +20,7 @@ export const DOC: WorkflowDefinition = { with: { connection: 'mailbox' }, }, ], - vars: [{ key: 'digest_to', value: 'me@dane.dev' }], + vars: [{ key: 'digest_to', t: 'text', value: 'me@dane.dev' }], steps: [ { id: 's2', diff --git a/source/packages/model/src/loops.test.ts b/source/packages/model/src/loops.test.ts new file mode 100644 index 0000000..8b4ed0f --- /dev/null +++ b/source/packages/model/src/loops.test.ts @@ -0,0 +1,179 @@ +import { coreFunctions, validate } from '@hatua/expressions' +import type { Step, WorkflowDefinition } from '@hatua/schema' +import { describe, expect, it } from 'vitest' +import { scopeFor } from './scope' +import { repeatSlot, setVarSlot, variableSlot, variableType } from './slots' + +/** + * `core.repeat` and `core.set_var`, on the side the rules corpus cannot reach. + * + * The corpus compares diagnostics; these follow the other half — what a Slot + * expects and what the checker then says about it. That is where the type + * marking lives, and it is the whole reason a var's type is declared: a + * `core.set_var` writing a number into a var the builder marked `text` would + * make every downstream check an answer to the wrong question. + * + * `sdk/go/slots_test.go` mirrors these assertion for assertion. + */ + +const doc = (over: Partial = {}): WorkflowDefinition => ({ + id: 'wf', + name: 'W', + version: 1, + status: 'draft', + steps: [], + ...over, +}) + +const setVar = (key: unknown, value: unknown): Step => ({ + id: 'bump', + use: 'core.set_var', + with: { key, value } as Record, +}) + +describe('a repeat’s condition', () => { + it('is a boolean, typed by the language rather than by a manifest', () => { + expect(repeatSlot('{{ var.done }}')).toEqual({ + name: 'until', + template: '{{ var.done }}', + expectedType: 'boolean', + }) + }) + + it('refuses a count where a condition belongs, which `with:` could not', () => { + const scope = scopeFor( + doc({ + vars: [{ key: 'seen', t: 'number', value: 0 }], + steps: [{ id: 'again', use: 'core.repeat', until: '{{ var.seen }}', steps: [] }], + }), + { board: null, id: 'again' }, + ) + const slot = repeatSlot('{{ var.seen }}') + + expect( + validate(slot.template, slot.expectedType, { scope, functions: coreFunctions() }), + ).not.toEqual([]) + expect( + validate('{{ var.seen > 3 }}', slot.expectedType, { scope, functions: coreFunctions() }), + ).toEqual([]) + }) +}) + +describe('what a set_var writes', () => { + const WORKFLOW = doc({ + vars: [{ key: 'attempt', t: 'number', value: 0 }], + steps: [setVar('attempt', '{{ 1 + 1 }}')], + }) + + it('is typed by the variable it names', () => { + expect(setVarSlot(WORKFLOW.steps[0] as Step, WORKFLOW.vars ?? [])).toEqual({ + name: 'value', + template: '{{ 1 + 1 }}', + expectedType: 'number', + }) + }) + + it('is not a Slot at all when the board declares no such variable', () => { + expect(setVarSlot(setVar('attemp', '1'), WORKFLOW.vars ?? [])).toBeNull() + expect(setVarSlot(setVar(undefined, '1'), WORKFLOW.vars ?? [])).toBeNull() + }) + + it('is not a Slot when the value is a literal rather than a Template', () => { + expect(setVarSlot(setVar('attempt', 7), WORKFLOW.vars ?? [])).toBeNull() + }) + + /** + * The end of the argument, from the document to a verdict. A var declared + * `boolean` refuses a number written into it, and the same document with `t: + * number` accepts it — so the marking the builder shows and the value the + * runner produces cannot disagree. + */ + it('is refused when it does not match the declaration, and accepted when it does', () => { + const asBoolean = doc({ + vars: [{ key: 'attempt', t: 'boolean', value: false }], + steps: [setVar('attempt', '{{ 1 + 1 }}')], + }) + const scope = scopeFor(asBoolean, { board: null, id: 'bump' }) + const wrong = setVarSlot(asBoolean.steps[0] as Step, asBoolean.vars ?? []) + expect(wrong).not.toBeNull() + expect( + validate(wrong?.template ?? '', wrong?.expectedType ?? 'text', { + scope, + functions: coreFunctions(), + }), + ).toEqual([expect.objectContaining({ code: 'EXPR_TYPE_MISMATCH' })]) + + const right = setVarSlot(WORKFLOW.steps[0] as Step, WORKFLOW.vars ?? []) + expect( + validate(right?.template ?? '', right?.expectedType ?? 'text', { + scope: scopeFor(WORKFLOW, { board: null, id: 'bump' }), + functions: coreFunctions(), + }), + ).toEqual([]) + }) + + /** + * A var declared on the wrong Board is out of reach rather than resolved + * differently, which is what makes `core.set_var` Board-scoped by + * construction rather than by a rule. + */ + it('cannot reach the workflow’s variables from inside a block', () => { + const inBlock = doc({ + vars: [{ key: 'attempt', t: 'number', value: 0 }], + blocks: [ + { + id: 'ask', + vars: [{ key: 'note', t: 'text', value: '' }], + steps: [setVar('attempt', '{{ 1 + 1 }}')], + }, + ], + }) + const block = inBlock.blocks?.[0] + expect(setVarSlot(block?.steps[0] as Step, block?.vars ?? [])).toBeNull() + }) +}) + +describe('a variable’s type', () => { + it('comes from its declaration, whatever the value beside it looks like', () => { + expect(variableType({ key: 'a', t: 'number', value: 'not a number' })).toBe('number') + expect(variableType({ key: 'a', t: 'boolean', value: '{{ run.tenant }}' })).toBe('boolean') + }) + + it('is unknown when nothing declares one, rather than guessed from the value', () => { + expect(variableType({ key: 'a', value: 7 } as never)).toBe('unknown') + }) + + it('checks the initial value, which nothing could before it was declared', () => { + expect(variableSlot({ key: 'attempt', t: 'number', value: '{{ 1 + 1 }}' })).toEqual({ + name: 'attempt', + template: '{{ 1 + 1 }}', + expectedType: 'number', + }) + // A literal is not a Template, so there is no Slot and nothing to check. + expect(variableSlot({ key: 'attempt', t: 'number', value: 0 })).toBeNull() + }) + + it('reaches the checker through scope, so `of:` shapes a member read', () => { + const scope = scopeFor( + doc({ + vars: [ + { + key: 'entry', + t: 'object', + of: [{ k: 'headline', label: 'Headline', t: 'text' }], + value: '', + }, + ], + steps: [{ id: 's1', use: 'component.email.send' }], + }), + { board: null, id: 's1' }, + ) + + expect( + validate('{{ var.entry.headline }}', 'text', { scope, functions: coreFunctions() }), + ).toEqual([]) + expect( + validate('{{ var.entry.headline }}', 'number', { scope, functions: coreFunctions() }), + ).not.toEqual([]) + }) +}) diff --git a/source/packages/model/src/scope.ts b/source/packages/model/src/scope.ts index d739202..e30a893 100644 --- a/source/packages/model/src/scope.ts +++ b/source/packages/model/src/scope.ts @@ -1,4 +1,4 @@ -import type { TypeNode, ValueType } from '@hatua/expressions' +import type { TypeNode } from '@hatua/expressions' import type { Block, ContextKey, @@ -6,11 +6,12 @@ import type { Manifest, Output, Step, + Variable, WorkflowDefinition, } from '@hatua/schema' import { TRIGGER_BUILTIN } from '@hatua/schema' import { blockIdOf, blockOf } from './blocks' -import { MAPPING_VERB, mapEntries } from './slots' +import { MAPPING_VERB, mapEntries, variableType } from './slots' import { type BoardId, boardOf, type StepRef } from './tree' /** @@ -199,7 +200,7 @@ export function boardScope( path: `var.${variable.key}`, kind: 'var', label: variable.key, - type: { type: varType(variable.value) }, + type: variableToType(variable), }) } @@ -270,19 +271,19 @@ const declarationMembers = (declarations: readonly Declaration[]): Record ({ /** The verb whose outputs come from its own configuration. */ export const MAPPING_VERB = 'core.map' +/** The verb that repeats its children until a condition holds. */ +export const REPEAT_VERB = 'core.repeat' + +/** The verb that iterates a collection. */ +export const FOR_EACH_VERB = 'core.for_each' + +/** The verb that branches. */ +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 Slot a `core.repeat`'s `until` resolves into. + * + * The mirror of `whenSlot`, and for the same reason: a condition is a boolean, + * and no manifest field can say so — `FIELD_KIND_TYPES` has no mappable boolean + * at all, because `bool` holds a literal rather than a Template. That is why + * `until` is a structural key beside `steps:` rather than a field under `with:`. + * Under `with:` it would type-check as text, so `{{ steps.s2.count }}` would + * pass as a termination condition. + * + * A repeat tests this AFTER its body, so the body always runs at least once. + */ +export const repeatSlot = (until: string): Slot => ({ + name: 'until', + template: until, + expectedType: 'boolean', +}) + +/** + * The type a variable's `{{ var. }}`, its initial `value` and every + * `core.set_var` writing it are all checked against. + * + * Declared rather than read off the value. A var is the one addressable thing + * whose content changes while the document does not, so inferring its type from + * the literal in the file would make the marking a lie the moment a + * `core.set_var` wrote something else — and every downstream check was answered + * against it (ADR-0013). + * + * `unknown` for a var carrying no `t` at all: the schema requires one, so this + * is a hand-edit, and refusing to check is the honest answer where guessing + * `text` would refuse a document over a type nothing declared. + */ +export const variableType = (variable: Variable): ValueType => + (variable.t as ValueType | undefined) ?? 'unknown' + +/** + * The Slot a `core.set_var`'s `value` resolves into, typed by the variable it + * names. + * + * The third verb a manifest cannot describe, alongside a call and a + * `core.return`, and for the same reason: what its field must produce is + * declared elsewhere in the document. Here it is the Board's `vars`, which is + * also why a `core.set_var` inside a Block can only ever name that Block's — + * `vars` is read from the Board the Step sits on, so there is no reaching out. + * + * Null when the step names no variable, or names one the Board does not + * declare: both have their own diagnostic, and resolving a Template against a + * type nothing declared would report a mismatch the user cannot act on. + */ +export function setVarSlot(step: Step, vars: readonly Variable[]): Slot | null { + const values = (step.with ?? {}) as Record + + const key = own(values, 'key') + if (typeof key !== 'string') return null + + const variable = vars.find((one) => one.key === key) + if (!variable) return null + + const template = own(values, 'value') + if (typeof template !== 'string') return null + + return { name: 'value', template, expectedType: variableType(variable) } +} + +/** + * The Slot a variable's initial value resolves into. + * + * A var's `value` may hold `{{ … }}`, and until `t` was declared there was + * nothing to check it against. Null for a literal: only a Template is a Slot. + */ +export function variableSlot(variable: Variable): Slot | null { + if (typeof variable.value !== 'string') return null + return { name: variable.key, template: variable.value, expectedType: variableType(variable) } +} + /** The `{key, value, type}` entries of a `map` field, ignoring anything malformed. */ export function mapEntries(value: unknown): MapEntry[] { if (!Array.isArray(value)) return [] diff --git a/source/packages/model/src/validity.ts b/source/packages/model/src/validity.ts index d1e11b9..c0ae31e 100644 --- a/source/packages/model/src/validity.ts +++ b/source/packages/model/src/validity.ts @@ -1,8 +1,16 @@ -import type { Block, Declaration, Manifest, Step, WorkflowDefinition } from '@hatua/schema' +import type { + Block, + Declaration, + Manifest, + Step, + Variable, + 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 { type BoardId, boards, own, stepKey, walkDocument, walkSteps } from './tree' +import { FOR_EACH_VERB, FORK_VERB, REPEAT_VERB, SET_VAR_VERB } from './slots' +import { type BoardId, boardOf, boards, own, stepKey, walkDocument, walkSteps } from './tree' /** * Whether a Workflow Definition is filled in enough to run — the rules that read @@ -164,6 +172,19 @@ export function missingRequiredFields( continue } + if (step.use === SET_VAR_VERB) { + // Structural, for the reason a return's fields are: what a `core.set_var` + // takes is a var key and a value typed by the var that key names, and no + // manifest knows which Board a Step is on. Its manifest declares no + // fields at all, so without this a set_var with nothing in it reports + // nothing. + for (const [key, label] of SET_VAR_FIELDS) { + if (!unfilled(own(values, key))) continue + out.push(raise('FIELD_REQUIRED', { ...subject, fieldKey: key }, { label })) + } + continue + } + fromManifest(subject, step.use, values) } @@ -232,7 +253,7 @@ export function malformedContainers(doc: WorkflowDefinition): Diagnostic[] { for (const { step, board } of walkDocument(doc)) { const subject: Partial = { stepId: step.id, ...boardOn(board) } - if (step.use === 'core.fork') { + if (step.use === FORK_VERB) { const branches = step.branches ?? [] // CONTEXT.md defines a Fork as "holding two or more Branches". One branch // is not a fork — it is the same path with a condition on it, which is @@ -255,29 +276,84 @@ export function malformedContainers(doc: WorkflowDefinition): Diagnostic[] { } } - if (step.use === 'core.for_each' && (step.steps ?? []).length === 0) { + // One code for both loop verbs: the mistake is the same and so is the fix, + // and the message names neither. + if ( + (step.use === FOR_EACH_VERB || step.use === REPEAT_VERB) && + (step.steps ?? []).length === 0 + ) { out.push(raise('LOOP_HAS_NO_BODY', subject)) } + + // Read from the tree rather than from `with:`, because that is where it + // lives: `FIELD_KIND_TYPES` has no mappable boolean, so a condition under + // `with:` would type-check as text — see `repeatSlot`. + if (step.use === REPEAT_VERB && (step.until ?? '').trim() === '') { + out.push(raise('REPEAT_HAS_NO_CONDITION', subject)) + } + + 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` + // against the Board would say no variable is called "undefined", which + // names a variable the user never wrote. + if (typeof key === 'string' && key !== '' && !varsOn(doc, board).some((v) => v.key === key)) { + out.push(raise('VAR_UNKNOWN', { ...subject, fieldKey: 'key' }, { name: key })) + } + } } return out } +/** + * The fields a `core.set_var` takes. Labels rather than keys in the message, + * matching every other required-field diagnostic — the sentence is read by + * someone looking at a form, not at YAML. + */ +const SET_VAR_FIELDS: readonly (readonly [string, string])[] = [ + ['key', 'Variable'], + ['value', 'Value'], +] + +/** + * The variables one Board declares: the workflow's at the root, a Block's + * inside one. + * + * This is the whole of "a `core.set_var` can never reach out of the Board it is + * on" — there is no second list to fall back to, so a Block naming a workflow + * variable is an unknown name rather than a scope the runner resolves + * differently. + */ +const varsOn = (doc: WorkflowDefinition, board: BoardId): readonly Variable[] => + board === null ? (doc.vars ?? []) : (boardOf(doc, board)?.block?.vars ?? []) + /** * Whether a step list, read from its own root level, always reaches a return. * * A `core.fork` discharges the obligation only when EVERY branch does, which is * the same all-paths reasoning `scopeFor` applies to what a Step can read. * - * A `core.for_each` never discharges it, and that is the load-bearing case: a - * return inside a loop body exits the Block early and is perfectly legal, but - * the list may be empty and the body may never run. That is the sibling-branch - * argument applied to time rather than to paths. + * The two loop verbs answer differently, and the difference is the whole rule. + * A `core.for_each` never discharges it: a return inside its body exits the + * Block early and is perfectly legal, but the list may be empty and the body + * may never run. A `core.repeat` does, because it tests its `until` after the + * body and therefore always runs it once. One question — is this region + * guaranteed to run at all — which is the sibling-branch argument applied to + * time rather than to paths. */ function alwaysReturns(steps: readonly Step[]): boolean { return steps.some((step) => { if (step.use === RETURN_VERB) return true - if (step.use !== 'core.fork') return false + + // A repeat tests its `until` AFTER the body, so the body always runs — and + // a region guaranteed to run discharges what it guarantees. This is the one + // line separating the two loop verbs, and it is the same question asked of + // both: a `core.for_each`'s list may be empty, a repeat's first pass is + // unconditional. + if (step.use === REPEAT_VERB) return alwaysReturns(step.steps ?? []) + + if (step.use !== FORK_VERB) return false const branches = step.branches ?? [] if (branches.length === 0) return false diff --git a/source/sdk/go/definition.go b/source/sdk/go/definition.go index 33903fc..9f735d1 100644 --- a/source/sdk/go/definition.go +++ b/source/sdk/go/definition.go @@ -84,11 +84,23 @@ type Trigger struct { With map[string]any `yaml:"with,omitempty"` } -// Variable is workflow-scoped mutable state. A list rather than a map so a type -// or label can be added later without breaking every existing file. +// Variable is Board-scoped mutable state: the workflow's at the root, a block's +// inside one. +// +// T is declared rather than read off Value, which is the decision core.set_var +// forced. Value is only the FIRST value — a core.set_var writes the same +// variable from a step — so a type inferred from the literal in the document +// would be a claim about one moment in an execution rather than about the +// variable, and every downstream check was answered against it (ADR-0013). +// +// NOT a Declaration: a declaration is a contract with nothing in it, and a +// variable's key is its own label. T and Of are spelled identically so one +// function reads both. type Variable struct { - Key string `yaml:"key"` - Value any `yaml:"value"` + Key string `yaml:"key"` + T string `yaml:"t"` + Of []Declaration `yaml:"of,omitempty"` + Value any `yaml:"value"` } // Step is one node of the tree. Steps nest through Branches (forks) and Steps @@ -101,6 +113,12 @@ type Step struct { With map[string]any `yaml:"with,omitempty"` Branches []Branch `yaml:"branches,omitempty"` Steps []Step `yaml:"steps,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 + // When is one: FieldKindTypes has no mappable boolean, so under With the + // condition would type-check as text. + Until string `yaml:"until,omitempty"` } // Branch is one labelled path of a fork. Order matters in a condition fork: @@ -174,16 +192,50 @@ func WalkDocument(d Definition, visit func(StepRef, Step)) { // BlockPrefix is the verb root that says a step calls a block in this document. const BlockPrefix = "block." -// ReturnVerb publishes a block's declared outputs and ends it. ForkVerb and -// ForEachVerb are the other two Hatua reads structurally: their shape is a -// position in the document rather than a field under `with:`, so no manifest -// can describe them. +// The verbs Hatua reads structurally, because their shape is a position in the +// document rather than a field under `with:` — so no manifest can describe them. +// +// ReturnVerb publishes a block's declared outputs and ends it. ForkVerb +// branches. ForEachVerb and RepeatVerb both nest, and differ in exactly one +// way that matters here: a repeat's body always runs and a for-each's may not. +// SetVarVerb writes one of its Board's variables, and is typed by that +// variable's declaration. const ( ReturnVerb = "core.return" ForkVerb = "core.fork" ForEachVerb = "core.for_each" + RepeatVerb = "core.repeat" + SetVarVerb = "core.set_var" ) +// VarsOn reports the variables one Board declares: the workflow's at the root, a +// block's inside one. +// +// This is the whole of "a core.set_var can never reach out of the Board it is +// on" — there is no second list to fall back to, so a block naming a workflow +// variable is an unknown name rather than a scope the runner resolves +// differently. +func VarsOn(doc Definition, board BoardID) []Variable { + if board == RootBoard { + return doc.Vars + } + if block := BlockOf(doc, board); block != nil { + return block.Vars + } + return nil +} + +// VariableOf returns one Board's variable by key, or nil. +func VariableOf(doc Definition, board BoardID, key string) *Variable { + vars := VarsOn(doc, board) + for i := range vars { + if vars[i].Key == key { + return &vars[i] + } + } + return nil +} + // CallsOf reports which blocks a step list reaches directly, in document order. // // Reads the whole board rather than its top level: a call nested inside a fork diff --git a/source/sdk/go/loops_test.go b/source/sdk/go/loops_test.go new file mode 100644 index 0000000..a8c3abd --- /dev/null +++ b/source/sdk/go/loops_test.go @@ -0,0 +1,173 @@ +package hatua + +import ( + "testing" + + "hatua.dev/go/expressions" +) + +// core.repeat and core.set_var, on the side the rules corpus cannot reach. +// +// The corpus compares diagnostics; these follow the other half — what a Slot +// expects and what the checker then says about it. That is where the type +// marking lives, and it is the whole reason a var's type is declared: a +// core.set_var writing a number into a var the builder marked `text` would make +// every downstream check an answer to the wrong question. +// +// packages/model/src/loops.test.ts mirrors these assertion for assertion. + +func setVarStep(with map[string]any) Step { + return Step{ID: "bump", Use: SetVarVerb, With: with} +} + +func TestRepeatConditionIsBoolean(t *testing.T) { + slot := RepeatSlot("{{ var.done }}") + if slot.Name != "until" || slot.ExpectedType != expressions.TypeBoolean { + t.Fatalf("expected a boolean `until` slot, got %+v", slot) + } +} + +func TestRepeatConditionRefusesACount(t *testing.T) { + doc := Definition{ + Vars: []Variable{{Key: "seen", T: "number", Value: 0}}, + Steps: []Step{{ID: "again", Use: RepeatVerb, Until: "{{ var.seen }}"}}, + } + ctx := expressions.CheckContext{ + Scope: ScopeFor(doc, StepRef{Board: RootBoard, ID: "again"}, nil, nil), + Functions: expressions.CoreFunctions(), + } + + if found := expressions.Validate("{{ var.seen }}", expressions.TypeBoolean, ctx); len(found) == 0 { + t.Fatalf("expected a count to be refused where a condition belongs") + } + if found := expressions.Validate("{{ var.seen > 3 }}", expressions.TypeBoolean, ctx); len(found) != 0 { + t.Fatalf("expected a comparison to pass, got %v", found) + } +} + +func TestSetVarSlotIsTypedByTheVariableItNames(t *testing.T) { + doc := Definition{ + Vars: []Variable{{Key: "attempt", T: "number", Value: 0}}, + Steps: []Step{setVarStep(map[string]any{"key": "attempt", "value": "{{ 1 + 1 }}"})}, + } + + slot, ok := SetVarSlot(doc, RootBoard, doc.Steps[0]) + if !ok { + t.Fatalf("expected a slot") + } + if slot.Name != "value" || slot.ExpectedType != expressions.TypeNumber { + t.Fatalf("expected a number `value` slot, got %+v", slot) + } +} + +func TestSetVarSlotIsAbsentWithoutAVariableToTypeIt(t *testing.T) { + doc := Definition{Vars: []Variable{{Key: "attempt", T: "number", Value: 0}}} + + cases := map[string]map[string]any{ + "a key no board declares": {"key": "attemp", "value": "1"}, + "no key at all": {"value": "1"}, + "a literal value": {"key": "attempt", "value": 7}, + } + for name, with := range cases { + if _, ok := SetVarSlot(doc, RootBoard, setVarStep(with)); ok { + t.Fatalf("%s: expected no slot", name) + } + } +} + +// The end of the argument, from the document to a verdict. A var declared +// `boolean` refuses a number written into it, and the same document with +// `t: number` accepts it — so the marking the builder shows and the value the +// runner produces cannot disagree. +func TestSetVarIsHeldToTheDeclaredType(t *testing.T) { + refused := Definition{ + Vars: []Variable{{Key: "attempt", T: "boolean", Value: false}}, + Steps: []Step{setVarStep(map[string]any{"key": "attempt", "value": "{{ 1 + 1 }}"})}, + } + slot, ok := SetVarSlot(refused, RootBoard, refused.Steps[0]) + if !ok { + t.Fatalf("expected a slot") + } + found := expressions.Validate(slot.Template, slot.ExpectedType, expressions.CheckContext{ + Scope: ScopeFor(refused, StepRef{Board: RootBoard, ID: "bump"}, nil, nil), + Functions: expressions.CoreFunctions(), + }) + if len(found) != 1 || found[0].Code != "EXPR_TYPE_MISMATCH" { + t.Fatalf("expected one EXPR_TYPE_MISMATCH, got %v", found) + } + + accepted := Definition{ + Vars: []Variable{{Key: "attempt", T: "number", Value: 0}}, + Steps: []Step{setVarStep(map[string]any{"key": "attempt", "value": "{{ 1 + 1 }}"})}, + } + slot, _ = SetVarSlot(accepted, RootBoard, accepted.Steps[0]) + found = expressions.Validate(slot.Template, slot.ExpectedType, expressions.CheckContext{ + Scope: ScopeFor(accepted, StepRef{Board: RootBoard, ID: "bump"}, nil, nil), + Functions: expressions.CoreFunctions(), + }) + if len(found) != 0 { + t.Fatalf("expected the declared type to accept it, got %v", found) + } +} + +// A var declared on the wrong Board is out of reach rather than resolved +// differently, which is what makes core.set_var Board-scoped by construction +// rather than by a rule. +func TestSetVarInsideABlockCannotReachTheWorkflowsVariables(t *testing.T) { + doc := Definition{ + Vars: []Variable{{Key: "attempt", T: "number", Value: 0}}, + Blocks: []Block{{ + ID: "ask", + Vars: []Variable{{Key: "note", T: "text", Value: ""}}, + Steps: []Step{setVarStep(map[string]any{"key": "attempt", "value": "{{ 1 + 1 }}"})}, + }}, + } + + if _, ok := SetVarSlot(doc, "ask", doc.Blocks[0].Steps[0]); ok { + t.Fatalf("expected the workflow's variable to be out of reach from inside a block") + } +} + +func TestVariableTypeComesFromItsDeclaration(t *testing.T) { + if got := variableType(Variable{Key: "a", T: "number", Value: "not a number"}); got != expressions.TypeNumber { + t.Fatalf("expected number, got %v", got) + } + // Nothing declares one, so nothing is guessed from the value beside it. + if got := variableType(Variable{Key: "a", Value: 7}); got != expressions.TypeUnknown { + t.Fatalf("expected unknown, got %v", got) + } +} + +func TestVariableSlotChecksTheInitialValue(t *testing.T) { + slot, ok := VariableSlot(Variable{Key: "attempt", T: "number", Value: "{{ 1 + 1 }}"}) + if !ok || slot.Name != "attempt" || slot.ExpectedType != expressions.TypeNumber { + t.Fatalf("expected a number slot named attempt, got %+v (%v)", slot, ok) + } + // A literal is not a Template, so there is no Slot and nothing to check. + if _, ok := VariableSlot(Variable{Key: "attempt", T: "number", Value: 0}); ok { + t.Fatalf("expected no slot for a literal") + } +} + +func TestVariableOfShapesAMemberRead(t *testing.T) { + doc := Definition{ + Vars: []Variable{{ + Key: "entry", + T: "object", + Of: []Declaration{{K: "headline", Label: "Headline", T: "text"}}, + Value: "", + }}, + Steps: []Step{{ID: "s1", Use: "component.email.send"}}, + } + ctx := expressions.CheckContext{ + Scope: ScopeFor(doc, StepRef{Board: RootBoard, ID: "s1"}, nil, nil), + Functions: expressions.CoreFunctions(), + } + + if found := expressions.Validate("{{ var.entry.headline }}", expressions.TypeText, ctx); len(found) != 0 { + t.Fatalf("expected a declared member to resolve, got %v", found) + } + if found := expressions.Validate("{{ var.entry.headline }}", expressions.TypeNumber, ctx); len(found) == 0 { + t.Fatalf("expected a text member to be refused where a number belongs") + } +} diff --git a/source/sdk/go/slots.go b/source/sdk/go/slots.go index f1829fb..9b6a723 100644 --- a/source/sdk/go/slots.go +++ b/source/sdk/go/slots.go @@ -1,9 +1,6 @@ package hatua import ( - "strings" - "time" - "hatua.dev/go/expressions" ) @@ -124,8 +121,8 @@ func declaredSlots(declarations []Declaration, step Step) []expressions.Slot { // SlotsForStep is the Slots any step resolves into, whichever kind it is. // -// One entry point so a runner never has to know that a call and a return are -// the two verbs a manifest cannot describe. +// One entry point so a runner never has to know that a call, a return and a +// core.set_var are the three verbs a manifest cannot describe. func SlotsForStep(doc Definition, board BoardID, step Step, manifest Manifest) []expressions.Slot { if called, ok := BlockIDOf(step.Use); ok { if block := BlockOf(doc, called); block != nil { @@ -139,9 +136,73 @@ func SlotsForStep(doc Definition, board BoardID, step Step, manifest Manifest) [ } return []expressions.Slot{} } + if step.Use == SetVarVerb { + if slot, ok := SetVarSlot(doc, board, step); ok { + return []expressions.Slot{slot} + } + return []expressions.Slot{} + } return SlotsFor(step, manifest) } +// RepeatSlot is the Slot a core.repeat's Until resolves into. +// +// The mirror of WhenSlot, and for the same reason: a condition is a boolean, and +// no manifest field can say so — FieldKindTypes has no mappable boolean at all, +// because `bool` holds a literal rather than a Template. That is why Until is a +// structural key beside Steps rather than a field under With; under With it +// would type-check as text, so `{{steps.s2.count}}` would pass as a termination +// condition. +// +// A repeat tests this AFTER its body, so the body always runs at least once. +func RepeatSlot(until string) expressions.Slot { + return expressions.Slot{Name: "until", Template: until, ExpectedType: expressions.TypeBoolean} +} + +// SetVarSlot is the Slot a core.set_var's `value` resolves into, typed by the +// variable it names. +// +// The third verb a manifest cannot describe, alongside a call and a core.return, +// and for the same reason: what its field must produce is declared elsewhere in +// the document. Here it is the Board's vars — which is also why a core.set_var +// inside a block can only ever name that block's, since the list is read from +// the Board the step sits on and there is no second one to fall back to. +// +// Not ok when the step names no variable, or names one the Board does not +// declare: both have their own diagnostic, and resolving a Template against a +// type nothing declared would report a mismatch the user cannot act on. +func SetVarSlot(doc Definition, board BoardID, step Step) (expressions.Slot, bool) { + key, ok := step.With["key"].(string) + if !ok { + return expressions.Slot{}, false + } + variable := VariableOf(doc, board, key) + if variable == nil { + return expressions.Slot{}, false + } + template, ok := step.With["value"].(string) + if !ok { + return expressions.Slot{}, false + } + return expressions.Slot{ + Name: "value", Template: template, ExpectedType: variableType(*variable), + }, true +} + +// VariableSlot is the Slot a variable's initial value resolves into. +// +// A var's Value may hold `{{ … }}`, and until T was declared there was nothing +// to check it against. Not ok for a literal: only a Template is a Slot. +func VariableSlot(variable Variable) (expressions.Slot, bool) { + template, ok := variable.Value.(string) + if !ok { + return expressions.Slot{}, false + } + return expressions.Slot{ + Name: variable.Key, Template: template, ExpectedType: variableType(variable), + }, true +} + // WhenSlot is the Slot a branch's condition resolves into. // // Separate from SlotsFor because a branch is not a step and has no manifest — @@ -318,7 +379,7 @@ func BoardScope(doc Definition, board BoardID, manifests []Manifest, context []C for _, variable := range vars { entries = append(entries, expressions.ScopeEntry{ Path: "var." + variable.Key, - Type: expressions.TypeNode{Type: varType(variable.Value)}, + Type: variableToType(variable), }) } @@ -409,35 +470,37 @@ func contextKeyType(key ContextKey) expressions.TypeNode { return node } -// varType reads a workflow variable's type from its literal value. +// variableType reads a variable's type from its declaration. // -// Vars are the one addressable thing with no declaration to consult, and calling -// them all unknown would make every `{{ var.x }}` in a workflow warn — which -// trains people to ignore warnings. A var holding text is text. A var holding a -// Template is genuinely unknown until it is evaluated, and says so. -func varType(value any) expressions.ValueType { - switch v := value.(type) { - case string: - if strings.Contains(v, "{{") { - return expressions.TypeUnknown +// Read from T rather than from the value beside it, which is the decision +// core.set_var forced: a var's value is only its FIRST value, so a type inferred +// from the literal in the document is a claim about one moment in an execution +// rather than about the variable (ADR-0013). It also settles a divergence the +// old inference had no answer for — yaml.v3 turns `value: 2024-01-01T00:00:00Z` +// into a time.Time while the builder's parser leaves it a string, so the two +// languages typed one scalar differently. A declared type is decoder-independent. +// +// Unknown for a var carrying no T at all: the schema requires one, so this is a +// hand-edit, and refusing to check is the honest answer where guessing `text` +// would refuse a document over a type nothing declared. +func variableType(variable Variable) expressions.ValueType { + if variable.T == "" { + return expressions.TypeUnknown + } + return expressions.ValueType(variable.T) +} + +// variableToType is that, plus the shape Of carries — spelled exactly as a +// declaration's Of is, so one traversal reads both. +func variableToType(variable Variable) expressions.TypeNode { + node := expressions.TypeNode{Type: variableType(variable)} + if len(variable.Of) > 0 { + node.Members = make(map[string]expressions.TypeNode, len(variable.Of)) + for _, member := range variable.Of { + node.Members[member.K] = declarationToType(member) } - return expressions.TypeText - case float64, float32, int, int8, int16, int32, int64, - uint, uint8, uint16, uint32, uint64: - return expressions.TypeNumber - case bool: - return expressions.TypeBoolean - case []any: - return expressions.TypeList - case time.Time: - // Text, not datetime, and the reason is the decoders rather than the - // language: yaml.v3 turns `value: 2024-01-01T00:00:00Z` into a - // time.Time, and the `yaml` package the builder uses leaves it a string. - // Typing it `datetime` here would block a publish in the Go SDK that the - // builder allows, over one scalar neither of them was told the type of. - return expressions.TypeText - } - return expressions.TypeUnknown + } + return node } // stepOutputType reports a step's outputs as a type. diff --git a/source/sdk/go/validity.go b/source/sdk/go/validity.go index 4ff8c03..dbd1fe8 100644 --- a/source/sdk/go/validity.go +++ b/source/sdk/go/validity.go @@ -223,6 +223,24 @@ func MissingRequiredFields(doc Definition, byUse map[string]Manifest) []Diagnost return } + if step.Use == SetVarVerb { + // Structural, for the reason a return's fields are: what a + // core.set_var takes is a var key and a value typed by the var that + // key names, and no manifest knows which Board a step is on. Its + // manifest declares no fields at all, so without this a set_var with + // nothing in it reports nothing. + for _, field := range setVarFields { + if !unfilled(values[field.key]) { + continue + } + subject.FieldKey = field.key + out = append(out, raise(CodeFieldRequired, subject, map[string]string{ + "label": field.label, + })) + } + return + } + fromManifest(subject, step.Use, values) }) @@ -233,6 +251,14 @@ func MissingRequiredFields(doc Definition, byUse map[string]Manifest) []Diagnost return out } +// setVarFields is what a core.set_var takes. Labels rather than keys in the +// message, matching every other required-field diagnostic — the sentence is read +// by someone looking at a form, not at YAML. +var setVarFields = []struct{ key, label string }{ + {"key", "Variable"}, + {"value", "Value"}, +} + // UnknownComponents reports a step or a trigger whose verb nothing declares. // // The two roots fail differently, so they are two codes. A `component.*` or @@ -319,9 +345,29 @@ func MalformedContainers(doc Definition) []Diagnostic { } } - if step.Use == ForEachVerb && len(step.Steps) == 0 { + // One code for both loop verbs: the mistake is the same and so is the + // fix, and the message names neither. + if (step.Use == ForEachVerb || step.Use == RepeatVerb) && len(step.Steps) == 0 { out = append(out, raise(CodeLoopHasNoBody, subject, nil)) } + + // Read from the tree rather than from With, because that is where it + // lives: FieldKindTypes has no mappable boolean, so a condition under + // With would type-check as text — see RepeatSlot. + if step.Use == RepeatVerb && strings.TrimSpace(step.Until) == "" { + out = append(out, raise(CodeRepeatHasNoCondition, subject, nil)) + } + + if step.Use == SetVarVerb { + key, named := step.With["key"].(string) + // A missing key is CodeFieldRequired's to report. Resolving an + // absent one against the Board would say no variable is called "" + // which names a variable the user never wrote. + if named && key != "" && VariableOf(doc, ref.Board, key) == nil { + subject.FieldKey = "key" + out = append(out, raise(CodeVarUnknown, subject, map[string]string{"name": key})) + } + } }) return out @@ -332,14 +378,25 @@ func MalformedContainers(doc Definition) []Diagnostic { // // A fork discharges the obligation only when EVERY branch does and the fork is // exhaustive — a falsy `when` on the last branch, matching how -// MalformedContainers reads one. A core.for_each never discharges it: the list -// may be empty and the body may never run, which is the sibling-branch argument -// applied to time rather than to paths. +// MalformedContainers reads one. +// +// The two loop verbs answer differently, and the difference is the whole rule. A +// core.for_each never discharges it: the list may be empty and the body may +// never run. A core.repeat does, because it tests its Until AFTER the body and +// therefore always runs it once. One question — is this region guaranteed to run +// at all — which is the sibling-branch argument applied to time rather than to +// paths. func alwaysReturns(steps []Step) bool { for _, step := range steps { if step.Use == ReturnVerb { return true } + if step.Use == RepeatVerb { + if alwaysReturns(step.Steps) { + return true + } + continue + } if step.Use != ForkVerb || len(step.Branches) == 0 { continue } diff --git a/source/sdk/js/src/expression.test.ts b/source/sdk/js/src/expression.test.ts index 4d66749..f7074e9 100644 --- a/source/sdk/js/src/expression.test.ts +++ b/source/sdk/js/src/expression.test.ts @@ -55,7 +55,7 @@ status: draft triggers: - { id: nightly, use: core.schedule, name: Nightly } vars: - - { key: digest_to, value: me@dane.dev } + - { key: digest_to, t: text, value: me@dane.dev } steps: - { id: s2, use: component.email.fetch, name: Fetch, with: { connection: mailbox } } - id: s6 diff --git a/source/sdk/js/src/index.ts b/source/sdk/js/src/index.ts index 5368d5f..7b26ad6 100644 --- a/source/sdk/js/src/index.ts +++ b/source/sdk/js/src/index.ts @@ -46,12 +46,16 @@ export { callSlots, cyclicBlocks, findStep, + repeatSlot, returnSlots, type StepRef, scopeFor, + setVarSlot, slotsFor, stepKey, upstreamOf, + variableSlot, + variableType, walkDocument, walkSteps, whenSlot, From 4d15b307a668ef6f66e655c76abd012492f38a00 Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Sun, 23 Aug 2026 08:13:30 +0100 Subject: [PATCH 03/10] feat(services,react): the type control, and the value box that stops being one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A variable row gains a Select for its declared type, and `addVariable` writes `t: text` rather than leaving it out — for the reason it mints a key rather than leaving one blank: the schema requires it, so a row without one is a document that stops projecting the moment it appears. The type control is now the one edit on the row that re-types every Expression reading the variable, and the value box is not. The value input gains an `expectedType` in exchange, which is the one place a variable's value is a Slot — everywhere else the variable is read rather than written, and until `t` existed there was nothing to check an initial value against. Round-trip covers both new document keys. An `until:` beside `steps:` and a `t:` inside `vars:` are structure the CST has to carry through untouched, and a key silently dropped there is a condition or a type marking that vanishes the first time anything edits the file. Every fixture carrying a var is rewritten. No fallback spelling, for the reason ADR-0014 gives. --- .../variable-key-is-not-an-identifier.yaml | 1 + .../conformance/definition/valid/blocks.yaml | 2 + source/conformance/definition/valid/full.yaml | 2 + .../packages/document/src/round-trip.test.ts | 54 +++++++++++++++++++ .../src/compounds/TemplateInput.stories.tsx | 4 +- .../react/src/compounds/TemplateInput.tsx | 6 +-- .../react/src/layouts/Workflow.module.css | 10 ++++ .../react/src/layouts/Workflow.stories.tsx | 2 + .../react/src/layouts/Workflow.test.tsx | 40 ++++++++++++-- .../packages/react/src/layouts/Workflow.tsx | 39 ++++++++++---- source/packages/schema/src/contract.test.ts | 2 +- source/packages/services/src/blocks.test.ts | 8 ++- source/packages/services/src/index.test.ts | 1 + .../packages/services/src/variables.test.ts | 44 +++++++++------ source/packages/services/src/variables.ts | 52 +++++++++++++----- 15 files changed, 218 insertions(+), 49 deletions(-) diff --git a/source/conformance/definition/invalid/variable-key-is-not-an-identifier.yaml b/source/conformance/definition/invalid/variable-key-is-not-an-identifier.yaml index 7654410..d890c51 100644 --- a/source/conformance/definition/invalid/variable-key-is-not-an-identifier.yaml +++ b/source/conformance/definition/invalid/variable-key-is-not-an-identifier.yaml @@ -8,5 +8,6 @@ version: 1 status: draft vars: - key: digest-to + t: text value: me@dane.dev steps: [] diff --git a/source/conformance/definition/valid/blocks.yaml b/source/conformance/definition/valid/blocks.yaml index 37a746a..ded843f 100644 --- a/source/conformance/definition/valid/blocks.yaml +++ b/source/conformance/definition/valid/blocks.yaml @@ -19,6 +19,7 @@ triggers: vars: - key: digest_to + t: text value: me@dane.dev blocks: @@ -53,6 +54,7 @@ blocks: # here: anything a block needs from the call site arrives as a parameter. vars: - key: subject_line + t: text value: "Digest: {{ params.entry.headline }}" params: - k: entry diff --git a/source/conformance/definition/valid/full.yaml b/source/conformance/definition/valid/full.yaml index 3bbeef0..70c3259 100644 --- a/source/conformance/definition/valid/full.yaml +++ b/source/conformance/definition/valid/full.yaml @@ -35,8 +35,10 @@ triggers: vars: - key: triage_date + t: datetime value: "{{ TRIGGER == 'nightly' ? triggers.nightly.triggered_at : triggers.on_mail.received_at }}" - key: digest_to + t: text value: me@dane.dev steps: diff --git a/source/packages/document/src/round-trip.test.ts b/source/packages/document/src/round-trip.test.ts index f3e6394..ad08f13 100644 --- a/source/packages/document/src/round-trip.test.ts +++ b/source/packages/document/src/round-trip.test.ts @@ -56,6 +56,7 @@ blocks: - { k: url, label: "Archive URL", t: text } vars: - key: attempt_note + t: text value: "" steps: - id: put @@ -94,6 +95,59 @@ describe('a document with blocks', () => { }) }) +/* + * A repeat and a set_var: the two keys neither a manifest nor `with:` holds. An + * `until:` sits beside `steps:` and a var carries a declared `t:`, so both are + * structure the CST has to carry through untouched — a key silently dropped + * here is a condition or a type marking that vanishes the first time anything + * edits the file. + */ +const WITH_A_LOOP = `id: wf_review +name: "Revision loop" +version: 2 +status: draft + +vars: + # Reset by the loop body, which is the cost ADR-0013 documents. + - key: approved + t: boolean + value: false + +steps: + - id: revise + use: core.repeat + until: "{{ var.approved }}" # tested after the body, so it always runs once + steps: + - id: draft + use: component.agent.act + - id: record + use: core.set_var + with: { key: approved, value: "{{ steps.draft.approved }}" } +` + +describe('a document with a repeat and a set_var', () => { + it('reproduces it byte for byte, the condition and its comment included', () => { + expect(parseWorkflow(WITH_A_LOOP).toString()).toBe(WITH_A_LOOP) + }) + + it('projects the condition beside the body rather than into `with:`', () => { + const doc = parseWorkflow(WITH_A_LOOP).toJSON() + const repeat = doc.steps[0] + + expect(repeat?.until).toBe('{{ var.approved }}') + expect(repeat?.with).toBeUndefined() + expect(repeat?.steps?.map((step) => step.id)).toEqual(['draft', 'record']) + }) + + it('projects a variable’s declared type, which nothing infers', () => { + expect(parseWorkflow(WITH_A_LOOP).toJSON().vars?.[0]).toEqual({ + key: 'approved', + t: 'boolean', + value: false, + }) + }) +}) + 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/react/src/compounds/TemplateInput.stories.tsx b/source/packages/react/src/compounds/TemplateInput.stories.tsx index 76d5ce6..a0bb35e 100644 --- a/source/packages/react/src/compounds/TemplateInput.stories.tsx +++ b/source/packages/react/src/compounds/TemplateInput.stories.tsx @@ -350,8 +350,8 @@ export const TypeMarkingInsideMixedText: Story = { } /** - * A workflow variable: no type marking at all, because `varType` reads a - * variable's type *from* its value and there is nothing to check it against. + * A Template nothing declares a type for: no marking at all, because there is + * nothing to check against and a rail that is always neutral is at least honest. */ export const NoDeclaredType: Story = { args: { value: '{{ run. }}', route: 'shortcut-inside', caret: 7 }, diff --git a/source/packages/react/src/compounds/TemplateInput.tsx b/source/packages/react/src/compounds/TemplateInput.tsx index ba23cee..c63d324 100644 --- a/source/packages/react/src/compounds/TemplateInput.tsx +++ b/source/packages/react/src/compounds/TemplateInput.tsx @@ -100,9 +100,9 @@ export interface TemplateInputProps { /** * The type the field declares, and what the left rail judges against. * - * Undefined where nothing declares one — a workflow variable, whose type is - * read *from* its value — and then no row is ever marked. There is nothing to - * check against, and a rail that is always neutral is at least honest. + * Optional because a caller may hold a Template nothing declares a type for, + * and then no row is ever marked. There is nothing to check against, and a + * rail that is always neutral is at least honest. */ expectedType?: ValueType /** diff --git a/source/packages/react/src/layouts/Workflow.module.css b/source/packages/react/src/layouts/Workflow.module.css index 1fc6574..c0b0d92 100644 --- a/source/packages/react/src/layouts/Workflow.module.css +++ b/source/packages/react/src/layouts/Workflow.module.css @@ -195,6 +195,16 @@ min-inline-size: 0; } + /* + * Its own row between the key and the value, rather than a third column in + * the head: the head is already a name box that must stay wide enough to read + * a key in, and squeezing a type box in beside the bin button makes both + * unreadable at the panel's width. + */ + .varType { + inline-size: 100%; + } + .add { display: grid; grid-template-columns: minmax(0, 1fr) auto; diff --git a/source/packages/react/src/layouts/Workflow.stories.tsx b/source/packages/react/src/layouts/Workflow.stories.tsx index 4938606..5c5b201 100644 --- a/source/packages/react/src/layouts/Workflow.stories.tsx +++ b/source/packages/react/src/layouts/Workflow.stories.tsx @@ -57,8 +57,10 @@ triggers: vars: # Where the digest goes. - key: digest_to + t: text value: "ops@example.com" - key: subject_prefix + t: text value: "[triage]" - key: threshold value: 10 diff --git a/source/packages/react/src/layouts/Workflow.test.tsx b/source/packages/react/src/layouts/Workflow.test.tsx index ff325ee..d715ca9 100644 --- a/source/packages/react/src/layouts/Workflow.test.tsx +++ b/source/packages/react/src/layouts/Workflow.test.tsx @@ -44,8 +44,10 @@ triggers: vars: # Where the digest goes. - key: digest_to + t: text value: "ops@example.com" - key: threshold + t: number value: 10 steps: @@ -834,10 +836,10 @@ describe('variables', () => { expect(source.writes[0]).toContain('{{ var.digest_to }}') }) - it('stores a value as what the text denotes, so the type follows the value', async () => { - // `varType` reads a variable's type off its value, so this box is also a - // type control — and typing `25` here has to mean the same as typing it in - // Text Mode (ADR-0001). + it('stores a value as what the text denotes, so Text Mode and this box agree', async () => { + // The type comes from `t`, but the value box still writes the scalar the + // text denotes: typing `25` here has to mean what typing it in Text Mode + // means (ADR-0001). const source = host() mount(source) @@ -846,6 +848,36 @@ describe('variables', () => { expect(source.writes[0]).toContain('value: 25') }) + /* + * The type control, which is the one edit on the row that re-types every + * Expression reading the variable. The value box does not: `core.set_var` + * writes the same variable from a Step, so the literal in the document is + * only what it starts as (ADR-0013). + */ + it('shows each variable’s declared type, and writes a change to it', async () => { + const source = host() + mount(source) + + const control = (await screen.findByLabelText('Type of threshold')) as HTMLSelectElement + expect(control.value).toBe('number') + + fireEvent.change(control, { target: { value: 'text' } }) + await waitFor(() => expect(source.writes).toHaveLength(1), AUTOSAVED) + expect(source.writes[0]).toContain('t: text') + // The value beside it is untouched, because the two say different things. + expect(source.writes[0]).toContain('value: 10') + }) + + it('gives a new variable a type, because the schema requires one', async () => { + const source = host() + mount(source) + + fireEvent.click(await screen.findByRole('button', { name: 'Add variable' })) + await waitFor(() => expect(source.writes).toHaveLength(1), AUTOSAVED) + expect(source.writes[0]).toContain('key: new_variable') + expect(source.writes[0]).toContain('t: text') + }) + it('stores a Template as a Template, holes and all', async () => { const source = host() mount(source) diff --git a/source/packages/react/src/layouts/Workflow.tsx b/source/packages/react/src/layouts/Workflow.tsx index 9b5574d..43f10d6 100644 --- a/source/packages/react/src/layouts/Workflow.tsx +++ b/source/packages/react/src/layouts/Workflow.tsx @@ -20,6 +20,7 @@ import { sequence, setTriggerField, setTriggerName, + setVariableType, setVariableValue, setWorkflowName, setWorkflowSlug, @@ -278,6 +279,7 @@ export function Workflow({ className, ...rest }: WorkflowProps) { onAdd={() => store?.apply(addVariable())} onRemove={(key) => store?.apply(removeVariable(key))} onRename={(from, to) => store?.apply(renameVariable(from, to))} + onType={(key, t) => store?.apply(setVariableType(key, t))} onValue={(key, value) => store?.apply(setVariableValue(key, value))} /> @@ -570,13 +572,18 @@ function TriggerCard({ ) } +/** The types a variable may declare, in the order the schema lists them. */ +const VARIABLE_TYPES = ['text', 'number', 'boolean', 'datetime', 'object', 'list'] as const + /** - * The workflow's variables: a key and a Template, per row. + * The workflow's variables: a key, a declared type and a Template, per row. * - * **A variable field is the one input with no type marking**, because `varType` - * infers a variable's type *from* its value. There is nothing to check it - * against — and editing one therefore changes what every downstream Expression - * reading it type-checks against, which is correct. + * **The type is declared, not read off the value**, so the value box is a + * Template input with a type marking like every other. `core.set_var` writes the + * same variable from a Step, which is what makes the literal in the document its + * FIRST value rather than its contract — a marking read off it would be a claim + * about one moment in an execution (ADR-0013). The type control is therefore the + * one edit on this row that re-types every Expression reading the variable. * * **Renaming a key does not rewrite References.** `{{ var.old_name }}` goes * stale and the checker reports it, exactly as it does for a Step that was @@ -590,6 +597,7 @@ function Variables({ onAdd, onRemove, onRename, + onType, onValue, }: { variables: readonly Variable[] @@ -597,6 +605,7 @@ function Variables({ onAdd: () => void onRemove: (key: string) => void onRename: (from: string, to: string) => void + onType: (key: string, t: string) => void onValue: (key: string, value: string) => void }) { return ( @@ -640,6 +649,18 @@ function Variables({ + onValue(variable.key, next)} /> diff --git a/source/packages/schema/src/contract.test.ts b/source/packages/schema/src/contract.test.ts index b156437..ee42ec9 100644 --- a/source/packages/schema/src/contract.test.ts +++ b/source/packages/schema/src/contract.test.ts @@ -21,7 +21,7 @@ const DEFINITION = { with: { cron: '0 7 * * 1-5' }, }, ], - vars: [{ key: 'digest_to', value: 'me@dane.dev' }], + vars: [{ key: 'digest_to', t: 'text', value: 'me@dane.dev' }], steps: [ { id: 's2', diff --git a/source/packages/services/src/blocks.test.ts b/source/packages/services/src/blocks.test.ts index 1c98abd..c195f60 100644 --- a/source/packages/services/src/blocks.test.ts +++ b/source/packages/services/src/blocks.test.ts @@ -40,6 +40,7 @@ triggers: vars: - key: digest_to + t: text value: "ops@example.com" steps: @@ -77,6 +78,7 @@ triggers: vars: - key: digest_to + t: text value: "ops@example.com" blocks: - id: archive_entry @@ -288,10 +290,12 @@ describe('a block’s own variables', () => { setVariableValue('attempt_note', 'first pass', 'archive'), ) - expect(projected(out).blocks?.[0]?.vars).toEqual([{ key: 'attempt_note', value: 'first pass' }]) + expect(projected(out).blocks?.[0]?.vars).toEqual([ + { key: 'attempt_note', t: 'text', value: 'first pass' }, + ]) // The workflow's own list is untouched, which is the lifetime difference: // a block's vars are rebuilt on every invocation. - expect(projected(out).vars).toEqual([{ key: 'digest_to', value: 'ops@example.com' }]) + expect(projected(out).vars).toEqual([{ key: 'digest_to', t: 'text', value: 'ops@example.com' }]) }) it('puts `vars:` before `steps:` inside the block', () => { diff --git a/source/packages/services/src/index.test.ts b/source/packages/services/src/index.test.ts index 31c6eff..6fcf323 100644 --- a/source/packages/services/src/index.test.ts +++ b/source/packages/services/src/index.test.ts @@ -62,6 +62,7 @@ const SURFACE = [ 'addVariable', 'removeVariable', 'renameVariable', + 'setVariableType', 'setVariableValue', ].sort() diff --git a/source/packages/services/src/variables.test.ts b/source/packages/services/src/variables.test.ts index 02cb09b..f53069a 100644 --- a/source/packages/services/src/variables.test.ts +++ b/source/packages/services/src/variables.test.ts @@ -2,7 +2,13 @@ import { parseWorkflow } from '@hatua/document' import { coreFunctions, validate } from '@hatua/expressions' import { scopeFor } from '@hatua/model' import { describe, expect, it } from 'vitest' -import { addVariable, removeVariable, renameVariable, setVariableValue } from './variables' +import { + addVariable, + removeVariable, + renameVariable, + setVariableType, + setVariableValue, +} from './variables' /** * The variable commands against a document directly. @@ -10,9 +16,9 @@ import { addVariable, removeVariable, renameVariable, setVariableValue } from '. * Two things are being protected. The first is the round trip: a variable is * added, renamed and removed out of a file that lives in the Host's repository, * and the comments, key order and quoting around it come back untouched - * (ADR-0001). The second is the consequence of editing one — `varType` reads a - * variable's type off its value, so a value box is also a type control, and the - * last test here follows that all the way to a verdict. + * (ADR-0001). The second is which edit re-types a variable: `t` is declared, so + * the type control moves every downstream verdict and the value box moves none, + * and the last tests here follow both all the way to a verdict. */ const SOURCE = `# The overnight triage. @@ -24,8 +30,10 @@ status: draft vars: # Where the digest goes. - key: digest_to + t: text value: "ops@example.com" - key: threshold + t: number value: 10 steps: @@ -186,11 +194,12 @@ describe('setVariableValue', () => { }) }) -describe('editing a variable changes what an Expression checks against', () => { +describe('retyping a variable changes what an Expression checks against', () => { /** - * The consequence of `varType`: a variable is the one addressable thing with - * no declaration to consult, so its type is read off its value. Editing one - * therefore re-types every Expression that reads it. + * The consequence of declaring `t`: the type control is what re-types every + * Expression reading the variable, and the value box is not — because a + * `core.set_var` writes the same variable from a Step, so the literal in the + * document is only what it starts as. * * Through `@hatua/expressions` over `scopeFor` output, which is where * expression type-checking happens. The validation store does none — it @@ -206,19 +215,22 @@ describe('editing a variable changes what an Expression checks against', () => { } it('goes from clean to reported when a number field starts reading text', () => { - const asNumber = apply(SOURCE, setVariableValue('threshold', '25')).toString() - expect(verdicts(asNumber, '{{ var.threshold }}', 'number')).toEqual([]) + expect(verdicts(SOURCE, '{{ var.threshold }}', 'number')).toEqual([]) - const asText = apply(SOURCE, setVariableValue('threshold', 'twenty five')).toString() + const asText = apply(SOURCE, setVariableType('threshold', 'text')).toString() expect(verdicts(asText, '{{ var.threshold }}', 'number')).not.toEqual([]) }) - it('clears the report when the value is edited back, in the same field', () => { - const asText = apply(SOURCE, setVariableValue('threshold', 'twenty five')).toString() - const reported = verdicts(asText, '{{ var.threshold }}', 'number') - expect(reported).toHaveLength(1) + it('clears the report when the type is set back, in the same control', () => { + const asText = apply(SOURCE, setVariableType('threshold', 'text')).toString() + expect(verdicts(asText, '{{ var.threshold }}', 'number')).toHaveLength(1) - const repaired = apply(asText, setVariableValue('threshold', '25')).toString() + const repaired = apply(asText, setVariableType('threshold', 'number')).toString() expect(verdicts(repaired, '{{ var.threshold }}', 'number')).toEqual([]) }) + + it('leaves the marking alone when only the value changes, because the value is only the first one', () => { + const written = apply(SOURCE, setVariableValue('threshold', 'twenty five')).toString() + expect(verdicts(written, '{{ var.threshold }}', 'number')).toEqual([]) + }) }) diff --git a/source/packages/services/src/variables.ts b/source/packages/services/src/variables.ts index 730250c..9709cd2 100644 --- a/source/packages/services/src/variables.ts +++ b/source/packages/services/src/variables.ts @@ -32,11 +32,12 @@ import type { EditCommand } from './command' * earlier variables, never a Step's outputs, because no Step is guaranteed to * have run. * - * Its *type* follows from what is stored: `varType` in @hatua/model reads a - * variable's type off its value, because a variable is the one addressable - * thing with no declaration to consult. So editing one changes what every - * downstream Expression reading it type-checks against, which is correct and is - * the reason `variables.test.ts` asserts it end to end. + * Its *type* is declared, in `t`, and is the one thing here that re-types every + * downstream Expression. The value box does not: `value` is only the FIRST + * value, because `core.set_var` writes the same variable from a Step, so a type + * read off the literal in the document would be a claim about one moment rather + * than about the variable (ADR-0013). `setVariableType` is therefore a command + * of its own, and `variables.test.ts` follows it to a verdict. */ /** @@ -91,7 +92,16 @@ export function addVariable(key?: string, board: BoardId = null): EditCommand { const listPath = ensureVars(document, board) const list = readAt(document, listPath) const index = Array.isArray(list) ? list.length : 0 - const value: Record = { key: key ?? mintKey(document, board), value: '' } + // `t` is written rather than left out, for the reason the key is minted + // rather than left blank: the schema requires it, so a row without one is + // a document that stops projecting the moment it appears. `text` because + // it is the type an empty value is, and it is the one every other type + // can be typed over from the row's control. + const value: Record = { + key: key ?? mintKey(document, board), + t: 'text', + value: '', + } insertNode(document, listPath, index, document.ast.createNode(value)) }, } @@ -147,6 +157,24 @@ export function renameVariable(from: string, to: string, board: BoardId = null): } } +/** + * Write a variable's declared type. + * + * The type control, and the only edit here that changes what a Template reading + * `{{ var. }}` is checked against. Separate from the value box because the + * two answer different questions once a `core.set_var` can write the variable: + * the value box says what it starts as, and this says what it must always be. + */ +export function setVariableType(key: string, t: string, board: BoardId = null): EditCommand { + return { + label: `Retype ${key}`, + apply(document) { + const listPath = varsPath(document, board) + setScalar(document, [...listPath, locateVariable(document, board, key), 't'], t) + }, + } +} + /** * The scalar a line of typed text denotes, by YAML's own rules. * @@ -154,8 +182,7 @@ export function renameVariable(from: string, to: string, board: BoardId = null): * Template, because `{{ … }}` is not a YAML scalar form — is text. That is not * a convenience: the same text typed into Text Mode produces exactly these * values, and a Workflow Definition edited two ways has to mean one thing - * (ADR-0001). It is also what makes the type marking move, since `varType` - * reads a variable's type off the value stored here. + * (ADR-0001). * * Only what round-trips. `007` and `1e400` are left as text rather than * normalised to `7` and `Infinity`, because rewriting what the user typed is @@ -170,13 +197,14 @@ const scalarFor = (text: string): string | number | boolean => { } /** - * Write a variable's value, as the Template it is. + * Write a variable's initial value, as the Template it is. * * `setIn` rather than an assignment onto the existing scalar, because this is * the one field whose *style* is part of its meaning: `value: "7"` is text and - * `value: 7` is a number, so keeping the quoting the previous value was written - * in would make a variable that starts out quoted impossible to turn into a - * number. Everywhere else the quoting is the user's and stays. + * `value: 7` is a number. What it is checked against comes from `t` rather than + * from the quoting, but a document round-trips what the user typed, and + * silently requoting a number as a string is the one thing a value box must not + * do. */ export function setVariableValue( key: string, From 0968b730bd498444cc471373171ce7221b37fe9e Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Sun, 23 Aug 2026 08:13:43 +0100 Subject: [PATCH 04/10] docs: record what core.repeat and core.set_var settled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0013 named both verbs and left their shape to the PR that gave them a reader. It now carries both, and resolves the deferral it named: a repeat tests after the body, so the body always runs, so it discharges a block's return obligation where a for_each cannot. The rejected alternatives are recorded because that is what an ADR is for — a pre-tested loop, iteration state on the container, a declared iteration bound, and the two ways of keeping the type inference alive. CONTEXT.md's Slot entry said the type is the field's, declared by the Component Manifest or by the language. A Block's params already made that incomplete and a declared var type makes it wrong: there are three sources, and a declaration in the document is one of them. Variable and Repeat get entries of their own. handoff.md built the Workflow tab on "a variable field is the one input with no type marking". That is false once a step can write the variable, and the section now says which control moves the marking and which one does not. --- CONTEXT.md | 26 +++++- docs/adr/0013-control-flow-nests.md | 118 +++++++++++++++++++++++++++- docs/handoff.md | 31 +++++--- 3 files changed, 159 insertions(+), 16 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index ef6492a..3a2f31c 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -97,11 +97,21 @@ _Avoid_: helper, macro, formula, built-in **Slot**: A named **Template** together with the type it must produce — `{name: "to", template: "{{ var.digest_to }}", expectedType: text}`. It names the *place* something is resolved into, which is -what distinguishes it from the **Template** it holds. The type is always the field's, declared by -the **Component Manifest** or — for a **Branch**'s `when` — by the language; it is never inferred -from the expression. +what distinguishes it from the **Template** it holds. **The type is never inferred from the +expression**, and it comes from one of three places: the **Component Manifest**'s field, a +declaration in the document (a **Block**'s `params`/`outputs`, a **Variable**'s `t`), or the +language — a **Branch**'s `when` and a **Repeat**'s `until` are boolean because a condition is. _Avoid_: binding, target, assignment, field value +**Variable**: +Named mutable state declared under a **Board**'s `vars:` and read anywhere on it as `{{var.}}`, +regardless of where it was written — which is what lets a **Repeat**'s body carry something back to +a **Step** that runs before it. **Its type is declared, in `t`, never read off its value**: `value` +is only the *initial* value, because `core.set_var` writes the same variable from a **Step**. A +Board's variables are its own — the workflow's at the root, a **Block**'s inside one, rebuilt on +every invocation — so a `core.set_var` can never reach out of the Board it is on. +_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 @@ -130,6 +140,16 @@ One labelled child path of a **Fork**, with an optional `when` condition and its Order is meaningful in a condition fork. _Avoid_: path, case, leg +**Repeat**: +A container **Step** (`core.repeat`) that runs its children, then evaluates its `until` condition, +and runs them again while that is false. **The body always runs at least once**, which is what +distinguishes it from `core.for_each` — a list may be empty — and what lets one discharge a +**Block**'s obligation to reach a `core.return`. `until` sits beside `steps:` rather than under +`with:`, because a **Component Manifest** field carries a rendering kind and cannot say "a +**Template** that must produce a boolean". It binds nothing: a counter is a **Variable**, written +by `core.set_var`. Nothing in the document bounds the iterations — a runner imposes its own ceiling. +_Avoid_: while, do-while, until-loop, retry + **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 4eb30e5..e420e61 100644 --- a/docs/adr/0013-control-flow-nests.md +++ b/docs/adr/0013-control-flow-nests.md @@ -83,7 +83,7 @@ that gives it a reader**, and amends this ADR when it does. - **`core.repeat`** — repeats its children until a condition holds. The gap `core.for_each` leaves: it iterates a collection, and nothing repeated on a condition. This is what "send it back for another revision" is, and what "ask whether to process another batch" is — the target of both is - the head of an enclosing container, which is why neither needs a jump. + the head of an enclosing container, which is why neither needs a jump. Its shape is below. - **`core.return`** — publishes a **Block**'s declared outputs and ends that Block. It is the mirror of `core.map`: the one component whose *inputs* no manifest can declare, because they are the enclosing Block's `outputs:`. @@ -195,10 +195,64 @@ the ordinary missing-field diagnostic. condition fork is first-match-wins, so one whose every branch is conditional can match none of them 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. 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 +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 +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. +## `core.repeat` tests after the body, and that decides three things + +```yaml +- id: revise + use: core.repeat + until: "{{ var.approved }}" + steps: + - { id: draft, use: component.agent.act } + - { id: record, use: core.set_var, with: { key: approved, value: "{{ steps.draft.ok }}" } } +``` + +**The body runs, then `until` is evaluated; false runs it again.** A pre-tested loop was the +alternative and is refused, because the two are not symmetric. A pre-tested loop is expressible as a +post-tested one whose body opens with a `core.fork` — a `core.return`, a future `core.stop`, or +simply nothing on the other branch. A post-tested loop is expressible as a pre-tested one only by +**duplicating the body** above the loop, which is the deduplication cost `blocks:` exists to pay +back, reintroduced by a control-flow choice. The motivating cases decide the same way: "send it back +for another revision" and "ask whether to process another batch" both have nothing to test until the +body has run once, so a pre-tested loop would make every use of one begin with a `core.set_var` +seeding a condition the user did not want to think about. + +**So a `core.repeat` discharges a Block's return obligation, and a `core.for_each` does not.** The +question `alwaysReturns` asks is only ever *is this region guaranteed to run at all* — a list may be +empty, a repeat's first pass cannot be skipped. That is the same reasoning that keeps sibling +branches out of scope, applied to time rather than to paths, and it now has one answer covering both +loop verbs rather than a special case for each. + +**`until:` is a structural key beside `steps:`, not a field under `with:`.** This is the wall +`blocks:` already hit from the other side: a Component Manifest field carries a rendering `kind` and +no type, so `slotsFor` recovers the expected type from `FIELD_KIND_TYPES`, and that vocabulary cannot +express "a Template that must produce a boolean" at all — `bool` holds a literal rather than a +Template. Under `with:` a condition would type-check as *text*, so `{{ steps.s2.count }}` would pass +as a termination condition and the half of the contract the field exists to carry would be gone. A +Branch's `when` sits in the same position for the same reason, and `repeatSlot` is `whenSlot` with a +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 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 +reading the file. Whether an `until` ever goes false is not: it depends on values that exist only +during a run. A `max:` written into the document would be a number Hatua could neither check nor +enforce, and a runner ignoring it would still be conformant, which is a promise the file does not +keep. **Bounding is the Host runner's obligation**: a runner imposes its own iteration ceiling and +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. + ## Loop state is a Board variable A repeated region usually has to carry something backwards — the reviewer's feedback reaching the @@ -206,6 +260,23 @@ draft step that runs before it. Nothing positional can do that: the writer runs it is not in scope. **`core.set_var` is the mechanism**, because a `var` is scoped to its **Board** and readable anywhere on it regardless of where it was written. +```yaml +- id: record + use: core.set_var + with: { key: approved, value: "{{ steps.draft.ok }}" } +``` + +**`key` names a variable on the Step's own Board**, and there is no second list to fall back to — +which is what makes "a Block's `core.set_var` can never reach the workflow's variables" true by +construction rather than by a rule. A key naming nothing the Board declares is a diagnostic, and it +blocks Publish rather than editing for the reason a stale `block.` does: renaming a variable is +ordinary building. + +**`value` is a Slot no manifest can type**, which makes `core.set_var` the third such verb beside a +call and a `core.return`. Its expected type is the named variable's, read where the variable is +declared — so a write that does not fit is the ordinary type diagnostic every other Slot already +produces, rather than a rule of its own. + A **Block** therefore declares `vars:` of its own, and `core.set_var` inside one writes those and can never reach out of the Board it is on. That is not a second concept: it is the same rule stated once, where "the Board" is the root for a Step in `steps:` and the Block for a Step inside one. @@ -214,6 +285,47 @@ The alternative was iteration state declared on `core.repeat` itself, initialise container, typed, and reset structurally on re-entry. It was rejected for costing a second concept where one already works. +## A variable's type is declared, because `core.set_var` made inference a lie + +`varType` read a variable's type off the literal beside it in the document, and the Workflow tab was +built on that: *"a variable field is the one input with no type marking"*. **That stops being true +the moment a Step can write the variable.** `value: ""` infers `text`; a `core.set_var` writing +`{{ 1 + 1 }}` into it makes the builder say `text` while the runner produces a number, and every +downstream answer — the type marking, the completion list's ranking, the Publish gate — was given +against a claim about one moment in an execution rather than about the variable. + +So **`vars` gains a required `t`, and an optional `of` for shape**, spelled exactly as a declaration +and a Run Context key are. The schema anticipated this in its own words — *"a list of key/value +objects rather than a map, so a `type` or `label` can be added later without a breaking change"* — +and ADR-0012's argument against inventing a second spelling for an idea the contract already has one +of holds here unchanged. A variable is still **not** a declaration: it carries a value, which no +declaration does, and its key is its own label, so three shared fields out of five is not one idea. + +`value` stops being the contract and becomes what it always was: the **initial** value. That is a +gain rather than a loss, because until `t` existed there was nothing to check an initial value +*against*, and a var seeded with `{{ … }}` was unchecked in both languages. + +Two alternatives were refused: + +- **Constrain `core.set_var` to the inferred type and report violations.** Keeps two mechanisms for + one idea, and makes the contract depend on how the first value happened to be written — a var + holding an object is unexpressible without an object literal, and an expression-valued var infers + `unknown`, so every write into it goes unchecked. +- **Weaken the inference to `unknown` for any var a `core.set_var` targets.** Makes the type marking + depend on a Step elsewhere in the document: adding a writer silently degrades every reader, so the + builder gets quieter exactly as the workflow gets more complicated. + +The cost is paid in full and is the one ADR-0014 already priced: **every existing document, fixture +and manifest is rewritten**, and `t` is required rather than defaulted, because a fallback spelling +is a second definition of the thing on the day it was declared. It also settles a divergence the +inference had no answer for — `yaml.v3` decodes `value: 2024-01-01T00:00:00Z` into a `time.Time` +while the builder's parser leaves it a string, so the two languages typed one scalar differently and +the Go SDK carried a comment saying so. A declared type is decoder-independent. + +The consequence for the builder is that the **type control**, not the value box, is what re-types +every Expression reading the variable. `CONTEXT.md`'s Slot entry and `docs/handoff.md`'s Workflow tab +are corrected to say so. + The cost is real and is documented rather than designed away: **a var written inside a loop survives into the next iteration of an enclosing loop**, so a workflow that must start each pass clean resets it explicitly. Nothing type-checks that reset. diff --git a/docs/handoff.md b/docs/handoff.md index 3fd3165..4239816 100644 --- a/docs/handoff.md +++ b/docs/handoff.md @@ -132,8 +132,9 @@ booleans. ### 3. Workflow variables -Rows of two mono `Input size="sm"` — key 118px, value flexible — plus a ghost trash button, then -`Button size="sm" variant="secondary" icon="plus"` **Add variable**. +Rows of a mono `Input size="sm"` for the key plus a ghost trash button, a full-width `Select` for +the declared type, and the value below, then `Button size="sm" variant="secondary" icon="plus"` +**Add variable**. **A variable's value is a Template**, not a literal. It may hold `{{ … }}`, so the value input is a [Template input](#the-template-input) like any other, and it gets the same completion. @@ -144,14 +145,24 @@ subset already exists inside `scopeFor`, which computes it before appending upst it as `boardScope(doc, board, manifests, runContext)` and let `scopeFor` be that plus the Steps, so the two readers share one definition. -**A variable field is the one input with no type marking**, because `varType` in `model/scope.ts` -infers a variable's type *from* its value. There is nothing to check it against. - -Editing a variable therefore changes what downstream Expressions type-check against. That is -correct, and it needs a test: change a variable from text to a number and a field reading it changes -verdict. It runs through `@hatua/expressions` with `scopeFor` output — not through the validation -store, which checks required fields, unknown components and malformed containers, and does no -expression type-checking at all. +**A variable declares its type, so the value input carries a type marking like every other.** The +type is read from `t` rather than from the value beside it, because `core.set_var` writes the same +variable from a Step — so the literal in the document is only what it *starts* as, and a marking +inferred from it would be a claim about one moment in an execution (ADR-0013). The value input's +`expectedType` is therefore the declared type, and it is the one place a variable's value is a Slot: +everywhere else the variable is read rather than written. + +**The type control is the one edit on the row that re-types downstream Expressions**, and the value +box is not. That needs a test on both halves: retyping `threshold` from number to text changes the +verdict of a number field reading it, and editing its value does not. It runs through +`@hatua/expressions` with `scopeFor` output — not through the validation store, which checks +required fields, unknown components and malformed containers, and does no expression type-checking +at all. + +A row's controls therefore map one to one onto commands: `renameVariable`, `setVariableType`, +`setVariableValue`, `removeVariable`. `addVariable` writes `t: text` rather than leaving it out, for +the reason it mints a key rather than leaving one blank — the schema requires it, so a row without +one is a document that stops projecting the moment it appears. #### Renaming a key From 8a6c7d5fd57d81a591b5634d793a14d436967dab Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Sun, 23 Aug 2026 08:26:28 +0100 Subject: [PATCH 05/10] docs: a repeat's condition is the fourth place a Template lives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Expression-editing table listed three sites and there are four. `until` sits in the same position a Branch's `when` does — a structural key on a container, typed boolean by the language rather than reached through a Component Manifest — so it is edited through the container that owns it, and whatever surface holds one holds the other. --- docs/handoff.md | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/docs/handoff.md b/docs/handoff.md index 4239816..94a7401 100644 --- a/docs/handoff.md +++ b/docs/handoff.md @@ -232,13 +232,20 @@ names is an *undiscriminated container arm*, which this does not have. The substantial half of the builder, and the part the original handoff specified least. -Three places hold a Template, and all three use the same widget: +Four places hold a Template, and all four use the same widget: | Site | Where it is edited | | --- | --- | | A Step's mappable `with:` fields, including `map` entries | Step editor | | A Branch's `when` | Step editor, via its Fork | -| A workflow variable's value | Workflow tab | +| A `core.repeat`'s `until` | Step editor, via its Repeat | +| A variable's value | Workflow tab | + +The two conditions are one row twice over. Both are a structural key on a container rather than a +field under `with:`, so neither is reached through the **Component Manifest** and both are typed +`boolean` by the language — which is why they are edited through the container that owns them rather +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. ### The Template input From 9e333ceda9fad1ef64897a5dfe8c8e00dc349355 Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Sun, 23 Aug 2026 12:28:50 +0100 Subject: [PATCH 06/10] fix(sdk/go): a variable's type is held to the set the schema declares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Validate()` checked a variable's key and stopped there, while a block's declarations went through the full contract. So `LoadDefinition` accepted `t: unknown`, `t: item` and `t: Text` on a var, and the JSON Schema refuses all three — a document the runner loads and the builder will not open, which is the divergence conformance/ exists to prevent. `unknown` and `item` are the sharp ones rather than the misspelling: the checker treats both as matching everything, so either switches the type gate off for that variable while the builder still draws a marking beside it. A var typed `unknown` reads as checked and is not. The type check is one function now, called from both the declaration path and the variable path, because a second copy is a second answer. A variable's `of` goes through `validateDeclarations` for the same reason: it carries the nested shape a declaration's does, so its members are held to the same contract rather than to none. The invalid fixture lands with it. Both languages run `conformance/definition/invalid/` and require rejection, so it pins the loader here and the zod gate there — the layer the rules corpus cannot see, because that corpus calls ValidateDefinition directly and never reaches Validate(). --- .../variable-type-is-not-a-declared-type.yaml | 16 ++++ source/sdk/go/load.go | 79 ++++++++++++++----- source/sdk/go/loops_test.go | 79 +++++++++++++++++++ 3 files changed, 153 insertions(+), 21 deletions(-) create mode 100644 source/conformance/definition/invalid/variable-type-is-not-a-declared-type.yaml diff --git a/source/conformance/definition/invalid/variable-type-is-not-a-declared-type.yaml b/source/conformance/definition/invalid/variable-type-is-not-a-declared-type.yaml new file mode 100644 index 0000000..372de44 --- /dev/null +++ b/source/conformance/definition/invalid/variable-type-is-not-a-declared-type.yaml @@ -0,0 +1,16 @@ +# expect: SCHEMA_INVALID +# +# A variable's type is what every `{{var.}}` read and every `core.set_var` +# write is checked against, so a type outside the declared set is not a narrower +# contract — it is no contract at all. `unknown` and `item` both match anything +# in the checker, so accepting either here would silently switch the type gate +# off for that variable while the builder still drew a marking beside it. +id: wf +name: W +version: 1 +status: draft +vars: + - key: attempt + t: unknown + value: 0 +steps: [] diff --git a/source/sdk/go/load.go b/source/sdk/go/load.go index fa72f56..f468670 100644 --- a/source/sdk/go/load.go +++ b/source/sdk/go/load.go @@ -136,10 +136,8 @@ func (d *Definition) Validate() error { return fmt.Errorf("%s: trigger %q needs a use", prefix, t.ID) } } - for _, v := range d.Vars { - if err := identifier(v.Key, "var key", prefix); err != nil { - return err - } + if err := validateVariables(d.Vars, "", prefix); err != nil { + return err } for _, b := range d.Blocks { if err := identifier(b.ID, "block id", prefix); err != nil { @@ -148,13 +146,11 @@ func (d *Definition) Validate() error { if b.Steps == nil { return fmt.Errorf("%s: block %q needs a steps list", prefix, b.ID) } - for _, v := range b.Vars { - if err := identifier(v.Key, "var key", prefix); err != nil { - return err - } + if err := validateVariables(b.Vars, fmt.Sprintf(" of block %q", b.ID), prefix); err != nil { + return err } for _, side := range [][]Declaration{b.Params, b.Outputs} { - if err := validateDeclarations(side, b.ID, prefix); err != nil { + if err := validateDeclarations(side, fmt.Sprintf("block %q", b.ID), prefix); err != nil { return err } } @@ -207,31 +203,72 @@ func identifier(value, what, prefix string) error { return nil } +// validateType holds a `t` to the set a document may name. +// +// One function rather than a switch per caller, because a block's contract and a +// Board's variables are checked against the same set and a second copy is a +// second answer: a type this accepts and the JSON Schema refuses is a document +// the runner loads and the builder will not open. +// +// `item` is refused along with everything else outside the set. It resolves by +// following a loop's list back to its source output, and neither a parameter nor +// a variable is the output of anything. `unknown` is refused for a sharper +// reason: the checker treats it as matching everything, so accepting it would +// switch the type gate off for that name while the builder still drew a marking +// beside it. +func validateType(t, what, prefix string) error { + switch t { + case "text", "number", "boolean", "datetime", "object", "list": + return nil + } + return fmt.Errorf("%s: %s declares an unusable type %q", prefix, what, t) +} + +// validateVariables holds a Board's variables to the same contract a block's +// declarations are held to. +// +// A variable's `t` is what every `{{var.}}` read and every `core.set_var` +// write is checked against, so it is the contract rather than a hint, and `of` +// carries the same nested shape a declaration's does. +func validateVariables(vars []Variable, where, prefix string) error { + for _, v := range vars { + if err := identifier(v.Key, "var key", prefix); err != nil { + return err + } + owner := fmt.Sprintf("variable %q%s", v.Key, where) + if err := validateType(v.T, owner, prefix); err != nil { + return err + } + if err := validateDeclarations(v.Of, owner, prefix); err != nil { + return err + } + } + return nil +} + // validateDeclarations holds a block's contract to the shape a manifest output -// has. `item` is refused: it resolves by following a loop's list back to its -// source, and a parameter is not the output of anything. +// has, and a variable's `of` to the same one. // // Deliberately unbounded in depth, because the schema is: a cap here and none in // the JSON Schema would refuse a document the builder published, which is the // divergence this function exists to prevent. A bound belongs in the shared // contract or nowhere. -func validateDeclarations(declarations []Declaration, block, prefix string) error { +func validateDeclarations(declarations []Declaration, owner, prefix string) error { for _, declaration := range declarations { if err := identifier(declaration.K, "declaration key", prefix); err != nil { return err } if declaration.Label == "" { - return fmt.Errorf("%s: %q in block %q needs a label", prefix, declaration.K, block) + return fmt.Errorf("%s: %q in %s needs a label", prefix, declaration.K, owner) } - switch declaration.T { - case "text", "number", "boolean", "datetime", "object", "list": - default: - return fmt.Errorf( - "%s: %q in block %q declares an unusable type %q", - prefix, declaration.K, block, declaration.T, - ) + if err := validateType( + declaration.T, + fmt.Sprintf("%q in %s", declaration.K, owner), + prefix, + ); err != nil { + return err } - if err := validateDeclarations(declaration.Of, block, prefix); err != nil { + if err := validateDeclarations(declaration.Of, owner, prefix); err != nil { return err } } diff --git a/source/sdk/go/loops_test.go b/source/sdk/go/loops_test.go index a8c3abd..c6860d3 100644 --- a/source/sdk/go/loops_test.go +++ b/source/sdk/go/loops_test.go @@ -171,3 +171,82 @@ func TestVariableOfShapesAMemberRead(t *testing.T) { t.Fatalf("expected a text member to be refused where a number belongs") } } + +// A variable's declared type is held to the same set a block's declarations are. +// +// The loader is a layer above the rules corpus — that corpus calls +// ValidateDefinition directly and never reaches Validate() — so a type the +// loader accepts and the JSON Schema refuses is a document this SDK loads and +// the builder will not open. `unknown` and `item` are the sharp cases: the +// checker treats both as matching everything, so accepting one switches the type +// gate off for that variable while the builder still draws a marking beside it. +func TestVariableTypeIsHeldToTheDeclaredSet(t *testing.T) { + document := func(declared string) string { + return "id: wf\nname: W\nversion: 1\nstatus: draft\n" + + "vars:\n - { key: attempt, t: " + declared + ", value: 0 }\nsteps: []\n" + } + + for _, declared := range []string{"unknown", "item", "Text", "str", ""} { + if _, err := LoadDefinition([]byte(document(declared))); err == nil { + t.Fatalf("expected t: %q to be refused", declared) + } + } + + for _, declared := range []string{"text", "number", "boolean", "datetime", "object", "list"} { + if _, err := LoadDefinition([]byte(document(declared))); err != nil { + t.Fatalf("expected t: %q to load, got: %v", declared, err) + } + } +} + +// A variable's `of` carries the same nested shape a declaration's does, so its +// members are held to the same contract rather than to none. +func TestVariableMembersAreHeldToTheSameContract(t *testing.T) { + const bad = `id: wf +name: W +version: 1 +status: draft +vars: + - key: entry + t: object + of: + - { k: headline, label: Headline, t: nonsense } + value: "" +steps: [] +` + if _, err := LoadDefinition([]byte(bad)); err == nil { + t.Fatalf("expected a member declaring an unusable type to be refused") + } + + const missingLabel = `id: wf +name: W +version: 1 +status: draft +vars: + - key: entry + t: object + of: + - { k: headline, t: text } + value: "" +steps: [] +` + if _, err := LoadDefinition([]byte(missingLabel)); err == nil { + t.Fatalf("expected a member with no label to be refused") + } + + const good = `id: wf +name: W +version: 1 +status: draft +vars: + - key: entry + t: object + of: + - { k: headline, label: Headline, t: text } + value: "" +steps: [] +` + if _, err := LoadDefinition([]byte(good)); err != nil { + t.Fatalf("expected a well-formed member to load, got: %v", err) + } +} From d59f4b1d763ccde0185cc8371f37cba66fe90258 Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Sun, 23 Aug 2026 12:29:03 +0100 Subject: [PATCH 07/10] docs: state variableType's reason as a property, and count the consequences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The doc comment on `variableType` named the reading it replaced, which is a sentence only a reader holding the previous version can act on. What matters is the property: a declared type is decoder-independent, and reading the value is not — yaml.v3 turns a timestamp scalar into a time.Time where the builder's parser leaves it a string, so the two languages disagree about one document. ADR-0013's section promised three consequences and argued four. --- docs/adr/0013-control-flow-nests.md | 2 +- source/sdk/go/slots.go | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/docs/adr/0013-control-flow-nests.md b/docs/adr/0013-control-flow-nests.md index e420e61..4027760 100644 --- a/docs/adr/0013-control-flow-nests.md +++ b/docs/adr/0013-control-flow-nests.md @@ -200,7 +200,7 @@ settles why. That is the same reasoning that keeps sibling branches out of scope 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. -## `core.repeat` tests after the body, and that decides three things +## `core.repeat` tests after the body, and that decides four things ```yaml - id: revise diff --git a/source/sdk/go/slots.go b/source/sdk/go/slots.go index 9b6a723..86195be 100644 --- a/source/sdk/go/slots.go +++ b/source/sdk/go/slots.go @@ -472,13 +472,15 @@ func contextKeyType(key ContextKey) expressions.TypeNode { // variableType reads a variable's type from its declaration. // -// Read from T rather than from the value beside it, which is the decision -// core.set_var forced: a var's value is only its FIRST value, so a type inferred -// from the literal in the document is a claim about one moment in an execution -// rather than about the variable (ADR-0013). It also settles a divergence the -// old inference had no answer for — yaml.v3 turns `value: 2024-01-01T00:00:00Z` -// into a time.Time while the builder's parser leaves it a string, so the two -// languages typed one scalar differently. A declared type is decoder-independent. +// Read from T rather than from the value beside it: a var's value is only its +// FIRST value, because core.set_var writes the same variable from a step, so a +// type read off the literal in the document is a claim about one moment in an +// execution rather than about the variable (ADR-0013). +// +// A declared type is also decoder-independent, which a type read off the value +// cannot be: yaml.v3 turns `value: 2024-01-01T00:00:00Z` into a time.Time while +// the builder's parser leaves it a string, so reading the value types one scalar +// two ways and the two languages disagree about the same document. // // Unknown for a var carrying no T at all: the schema requires one, so this is a // hand-edit, and refusing to check is the honest answer where guessing `text` From 229ff651ee14921c52e79857d60d869597de03e2 Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Sun, 23 Aug 2026 13:06:23 +0100 Subject: [PATCH 08/10] fix(react): every story fixture projects, and a test that says so MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two vars in the Workflow stories carried no `t`, so the whole document failed to project — and a region handed a failed projection draws the state it shows a Host that wired nothing up. Every Workflow story rendered that instead of the builder, including the variables panel holding the type control. Nothing caught it because nothing runs a story fixture: Storybook renders it by hand, no test mounts it, and the file still compiles. That makes a story the one document in the repo a schema change can break silently, which is the failure worth pinning rather than the two missing keys. The guard reads the fixtures out of the story sources and projects each through @hatua/document, which is the path the editing store takes — a test-only dependency, and nothing under src/ outside that file imports it. It reads source rather than the module, so it undoes the escapes the compiler would have: a fixture written with `\n` holds two characters where the running story holds a newline. --- source/packages/react/package.json | 1 + .../react/src/layouts/Workflow.stories.tsx | 2 + .../src/layouts/stories.fixtures.test.ts | 81 +++++++++++++++++++ source/pnpm-lock.yaml | 3 + 4 files changed, 87 insertions(+) create mode 100644 source/packages/react/src/layouts/stories.fixtures.test.ts diff --git a/source/packages/react/package.json b/source/packages/react/package.json index c300baf..6f5467d 100644 --- a/source/packages/react/package.json +++ b/source/packages/react/package.json @@ -37,6 +37,7 @@ "@hatua/services": "workspace:*" }, "devDependencies": { + "@hatua/document": "workspace:*", "@storybook/react-vite": "^10.5.8", "@testing-library/react": "^16.3.0", "@types/react": "^19.2.2", diff --git a/source/packages/react/src/layouts/Workflow.stories.tsx b/source/packages/react/src/layouts/Workflow.stories.tsx index 5c5b201..518b9f9 100644 --- a/source/packages/react/src/layouts/Workflow.stories.tsx +++ b/source/packages/react/src/layouts/Workflow.stories.tsx @@ -63,8 +63,10 @@ vars: t: text value: "[triage]" - key: threshold + t: number value: 10 - key: greeting + t: text value: "Morning, {{ triggers.t1.owner }}" steps: diff --git a/source/packages/react/src/layouts/stories.fixtures.test.ts b/source/packages/react/src/layouts/stories.fixtures.test.ts new file mode 100644 index 0000000..1fd1a35 --- /dev/null +++ b/source/packages/react/src/layouts/stories.fixtures.test.ts @@ -0,0 +1,81 @@ +/** biome-ignore-all lint/correctness/noNodejsModules: a Node test reading story sources from disk; nothing here ships to a browser. */ +import { readdirSync, readFileSync } from 'node:fs' +import { join } from 'node:path' +import { parseWorkflow } from '@hatua/document' +import { describe, expect, it } from 'vitest' + +/** + * Every Workflow Definition a story hands a region has to project. + * + * A story fixture is the only document in the repo nothing executes: Storybook + * renders it by hand and no test mounts it, so a fixture that stops satisfying + * the schema does not fail — the region simply falls back to the state it shows + * a Host that wired nothing up, and every story quietly draws the wrong screen. + * That is invisible in review precisely because the file still compiles. + * + * A field added to the schema is what makes this reachable: it lands in the + * fixtures under test and in the fixtures under `stories`, and only the first + * set has anything watching it. + * + * Through `@hatua/document` rather than the schema directly, because that is the + * path a region's document actually takes — the editing store parses the Host's + * text and hands the region `projection.success ? projection.data : null`, and + * `null` is what draws the wrong screen. A test-only dependency: nothing under + * `src/` outside this file imports it, and the layering rule in this + * directory's README still stands. + */ + +const LAYOUTS = import.meta.dirname + +/** + * Template literals holding a workflow document, by the one marker every one of + * them carries: a top-level `id:` before any nesting. Narrow on purpose — a + * story may hold YAML that is a manifest or a fragment, and neither is this + * schema's to judge. + */ +function documentsIn(source: string): string[] { + const found: string[] = [] + for (const match of source.matchAll(/`([^`]*)`/g)) { + const body = unescaped(match[1] ?? '') + if (/^(?:#[^\n]*\n|\s*\n)*id:\s*\S/.test(body)) found.push(body) + } + return found +} + +/** + * What the compiler would have made of the literal. + * + * Read from source rather than from the module, so the escapes are still text: + * a fixture written `id: wf\nsteps: []` holds two characters where the running + * story holds a newline, and feeding that to a YAML parser fails for a reason + * that has nothing to do with the fixture. One pass, so a literal backslash + * cannot be re-read as the start of the next escape. + */ +const unescaped = (body: string): string => + body.replace(/\\([nrt\\`$])/g, (_, char: string) => + char === 'n' ? '\n' : char === 'r' ? '\r' : char === 't' ? '\t' : char, + ) + +describe('story fixtures', () => { + const files = readdirSync(LAYOUTS).filter((name) => name.endsWith('.stories.tsx')) + expect(files.length).toBeGreaterThan(0) + + const documents = files.flatMap((file) => + documentsIn(readFileSync(join(LAYOUTS, file), 'utf8')).map( + (source, index) => [`${file} · document ${index + 1}`, source] as const, + ), + ) + + // A guard that found nothing is a guard that protects nothing. + it('finds the documents the stories hold', () => { + expect(documents.length).toBeGreaterThan(0) + }) + + for (const [name, source] of documents) { + it(`${name} projects`, () => { + const projected = parseWorkflow(source).validate() + expect(projected.error?.issues).toBeUndefined() + expect(projected.success).toBe(true) + }) + } +}) diff --git a/source/pnpm-lock.yaml b/source/pnpm-lock.yaml index f95bdf0..29e3314 100644 --- a/source/pnpm-lock.yaml +++ b/source/pnpm-lock.yaml @@ -117,6 +117,9 @@ importers: specifier: workspace:* version: link:../services devDependencies: + '@hatua/document': + specifier: workspace:* + version: link:../document '@storybook/react-vite': specifier: ^10.5.8 version: 10.5.8(@types/react-dom@19.2.4(@types/react@19.2.18))(@types/react@19.2.18)(esbuild@0.28.2)(react-dom@19.2.8(react@19.2.8))(react@19.2.8)(rollup@4.62.4)(storybook@10.5.8(@types/react@19.2.18)(react@19.2.8))(typescript@7.0.2)(vite@8.2.1(@types/node@26.2.0)(esbuild@0.28.2)(yaml@2.9.0)) From 6d1a39be58fcf144798d0469ee68aadabf960af8 Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Sun, 23 Aug 2026 13:06:25 +0100 Subject: [PATCH 09/10] fix(model): state the set_var Board rule once, and agree with Go about an empty type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `setVarSlot` took a list of variables, so the caller decided whether a Block falls back to the workflow's — the one rule this verb exists inside. It now takes the Board, matching `SetVarSlot` in the Go SDK argument for argument, and `varsOn` moves to `tree.ts` beside `boardOf` and is exported: a runner has to answer the same question the builder does, and a rule restated at two call sites is two rules the day one of them gains a fallback. `variableType` used `??`, which passes an empty string through, while Go tests `T == ""`. A hand-edited `t: ""` therefore typed as `''` here — matching nothing, so every read of that variable reported a mismatch — and as `unknown` there, matching everything. Same document, two verdicts. The JS SDK re-exports `varsOn`, `variableOn` and the two verb constants, so a runner can dispatch and resolve without restating either. --- source/packages/model/src/loops.test.ts | 18 +++++++++++------- source/packages/model/src/slots.ts | 23 +++++++++++++++-------- source/packages/model/src/tree.ts | 24 +++++++++++++++++++++++- source/packages/model/src/validity.ts | 23 ++--------------------- source/sdk/js/src/index.ts | 4 ++++ 5 files changed, 55 insertions(+), 37 deletions(-) diff --git a/source/packages/model/src/loops.test.ts b/source/packages/model/src/loops.test.ts index 8b4ed0f..0b1d885 100644 --- a/source/packages/model/src/loops.test.ts +++ b/source/packages/model/src/loops.test.ts @@ -66,7 +66,7 @@ describe('what a set_var writes', () => { }) it('is typed by the variable it names', () => { - expect(setVarSlot(WORKFLOW.steps[0] as Step, WORKFLOW.vars ?? [])).toEqual({ + expect(setVarSlot(WORKFLOW, null, WORKFLOW.steps[0] as Step)).toEqual({ name: 'value', template: '{{ 1 + 1 }}', expectedType: 'number', @@ -74,12 +74,12 @@ describe('what a set_var writes', () => { }) it('is not a Slot at all when the board declares no such variable', () => { - expect(setVarSlot(setVar('attemp', '1'), WORKFLOW.vars ?? [])).toBeNull() - expect(setVarSlot(setVar(undefined, '1'), WORKFLOW.vars ?? [])).toBeNull() + expect(setVarSlot(WORKFLOW, null, setVar('attemp', '1'))).toBeNull() + expect(setVarSlot(WORKFLOW, null, setVar(undefined, '1'))).toBeNull() }) it('is not a Slot when the value is a literal rather than a Template', () => { - expect(setVarSlot(setVar('attempt', 7), WORKFLOW.vars ?? [])).toBeNull() + expect(setVarSlot(WORKFLOW, null, setVar('attempt', 7))).toBeNull() }) /** @@ -94,7 +94,7 @@ describe('what a set_var writes', () => { steps: [setVar('attempt', '{{ 1 + 1 }}')], }) const scope = scopeFor(asBoolean, { board: null, id: 'bump' }) - const wrong = setVarSlot(asBoolean.steps[0] as Step, asBoolean.vars ?? []) + const wrong = setVarSlot(asBoolean, null, asBoolean.steps[0] as Step) expect(wrong).not.toBeNull() expect( validate(wrong?.template ?? '', wrong?.expectedType ?? 'text', { @@ -103,7 +103,7 @@ describe('what a set_var writes', () => { }), ).toEqual([expect.objectContaining({ code: 'EXPR_TYPE_MISMATCH' })]) - const right = setVarSlot(WORKFLOW.steps[0] as Step, WORKFLOW.vars ?? []) + const right = setVarSlot(WORKFLOW, null, WORKFLOW.steps[0] as Step) expect( validate(right?.template ?? '', right?.expectedType ?? 'text', { scope: scopeFor(WORKFLOW, { board: null, id: 'bump' }), @@ -129,7 +129,7 @@ describe('what a set_var writes', () => { ], }) const block = inBlock.blocks?.[0] - expect(setVarSlot(block?.steps[0] as Step, block?.vars ?? [])).toBeNull() + expect(setVarSlot(inBlock, 'ask', block?.steps[0] as Step)).toBeNull() }) }) @@ -141,6 +141,10 @@ describe('a variable’s type', () => { it('is unknown when nothing declares one, rather than guessed from the value', () => { expect(variableType({ key: 'a', value: 7 } as never)).toBe('unknown') + // Empty and absent alike, matching the Go SDK. A `t: ""` reaching here is a + // hand-edit like a missing key is, and treating it as a type nothing + // declares would report a mismatch on every read of the variable. + expect(variableType({ key: 'a', t: '', value: 7 } as never)).toBe('unknown') }) it('checks the initial value, which nothing could before it was declared', () => { diff --git a/source/packages/model/src/slots.ts b/source/packages/model/src/slots.ts index 9f80c03..7af2558 100644 --- a/source/packages/model/src/slots.ts +++ b/source/packages/model/src/slots.ts @@ -1,7 +1,7 @@ import type { Slot, ValueType } from '@hatua/expressions' -import type { Manifest, Step, Variable } from '@hatua/schema' +import type { Manifest, Step, Variable, WorkflowDefinition } from '@hatua/schema' import { isMappable, type MAPPABLE_FIELD_KINDS } from '@hatua/schema' -import { own } from './tree' +import { type BoardId, own, variableOn } from './tree' /** * The bridge between a Component Manifest and the expression language. @@ -142,12 +142,14 @@ export const repeatSlot = (until: string): Slot => ({ * `core.set_var` wrote something else — and every downstream check was answered * against it (ADR-0013). * - * `unknown` for a var carrying no `t` at all: the schema requires one, so this - * is a hand-edit, and refusing to check is the honest answer where guessing - * `text` would refuse a document over a type nothing declared. + * `unknown` for a var carrying no `t` at all — absent or empty alike, matching + * the Go SDK, because a hand-edit is exactly what reaches here and `t: ""` is as + * plausible a one as a missing key. The schema requires a type, so refusing to + * check is the honest answer where guessing `text` would refuse a document over + * a type nothing declared. */ export const variableType = (variable: Variable): ValueType => - (variable.t as ValueType | undefined) ?? 'unknown' + variable.t ? (variable.t as ValueType) : 'unknown' /** * The Slot a `core.set_var`'s `value` resolves into, typed by the variable it @@ -159,17 +161,22 @@ export const variableType = (variable: Variable): ValueType => * also why a `core.set_var` inside a Block can only ever name that Block's — * `vars` is read from the Board the Step sits on, so there is no reaching out. * + * Takes the Board rather than a list of variables, so the caller cannot supply + * the wrong one: the Go SDK's `SetVarSlot` has the same signature, and a runner + * handed a list would be the one deciding whether a Block falls back to the + * workflow's variables — which is the rule this verb exists inside. + * * Null when the step names no variable, or names one the Board does not * declare: both have their own diagnostic, and resolving a Template against a * type nothing declared would report a mismatch the user cannot act on. */ -export function setVarSlot(step: Step, vars: readonly Variable[]): Slot | null { +export function setVarSlot(doc: WorkflowDefinition, board: BoardId, step: Step): Slot | null { const values = (step.with ?? {}) as Record const key = own(values, 'key') if (typeof key !== 'string') return null - const variable = vars.find((one) => one.key === key) + const variable = variableOn(doc, board, key) if (!variable) return null const template = own(values, 'value') diff --git a/source/packages/model/src/tree.ts b/source/packages/model/src/tree.ts index 31e0421..52d1af6 100644 --- a/source/packages/model/src/tree.ts +++ b/source/packages/model/src/tree.ts @@ -1,4 +1,4 @@ -import type { Block, Step, WorkflowDefinition } from '@hatua/schema' +import type { Block, Step, Variable, WorkflowDefinition } from '@hatua/schema' /** * Pure domain rules over the step tree. No state, no I/O, no YAML — those live @@ -108,6 +108,28 @@ export function findStep(doc: WorkflowDefinition, ref: StepRef): Step | undefine return undefined } +/** + * The variables one Board declares: the workflow's at the root, a Block's inside + * one. + * + * This is the whole of "a `core.set_var` can never reach out of the Board it is + * on" — there is no second list to fall back to, so a Block naming a workflow + * variable is an unknown name rather than a scope a runner resolves differently. + * + * Exported because a runner has to answer the same question the builder does, + * and the Go SDK's `VarsOn` is this function: a rule restated at two call sites + * is two rules the day one of them gains a fallback. + */ +export const varsOn = (doc: WorkflowDefinition, board: BoardId): readonly Variable[] => + board === null ? (doc.vars ?? []) : (boardOf(doc, board)?.block?.vars ?? []) + +/** One Board's variable by key, or undefined when that Board declares none. */ +export const variableOn = ( + doc: WorkflowDefinition, + board: BoardId, + key: string, +): Variable | undefined => varsOn(doc, board).find((variable) => variable.key === key) + /** Every step id on one Board, for detecting references to steps that vanished. */ export function stepIds(doc: WorkflowDefinition, board: BoardId): Set { const found = boardOf(doc, board) diff --git a/source/packages/model/src/validity.ts b/source/packages/model/src/validity.ts index c0ae31e..6949204 100644 --- a/source/packages/model/src/validity.ts +++ b/source/packages/model/src/validity.ts @@ -1,16 +1,9 @@ -import type { - Block, - Declaration, - Manifest, - Step, - Variable, - WorkflowDefinition, -} from '@hatua/schema' +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 { type BoardId, boardOf, boards, own, stepKey, walkDocument, walkSteps } from './tree' +import { type BoardId, boards, own, stepKey, varsOn, walkDocument, walkSteps } from './tree' /** * Whether a Workflow Definition is filled in enough to run — the rules that read @@ -316,18 +309,6 @@ const SET_VAR_FIELDS: readonly (readonly [string, string])[] = [ ['value', 'Value'], ] -/** - * The variables one Board declares: the workflow's at the root, a Block's - * inside one. - * - * This is the whole of "a `core.set_var` can never reach out of the Board it is - * on" — there is no second list to fall back to, so a Block naming a workflow - * variable is an unknown name rather than a scope the runner resolves - * differently. - */ -const varsOn = (doc: WorkflowDefinition, board: BoardId): readonly Variable[] => - board === null ? (doc.vars ?? []) : (boardOf(doc, board)?.block?.vars ?? []) - /** * Whether a step list, read from its own root level, always reaches a return. * diff --git a/source/sdk/js/src/index.ts b/source/sdk/js/src/index.ts index 7b26ad6..5c32e53 100644 --- a/source/sdk/js/src/index.ts +++ b/source/sdk/js/src/index.ts @@ -46,16 +46,20 @@ export { callSlots, cyclicBlocks, findStep, + REPEAT_VERB, repeatSlot, returnSlots, + SET_VAR_VERB, type StepRef, scopeFor, setVarSlot, slotsFor, stepKey, upstreamOf, + variableOn, variableSlot, variableType, + varsOn, walkDocument, walkSteps, whenSlot, From 99ebc2e0c4b8ad37fed1fd4aaa01fd4fc4c1b5b8 Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Sun, 23 Aug 2026 18:20:59 +0100 Subject: [PATCH 10/10] docs: say what a declared type does on screen, which is rail the picker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three places claimed a variable's value box carries a type marking. Looking at it in Storybook says otherwise: `expectedType` reaches `expectedAt` and from there the completion list, so it decides which candidate rows are railed as fitting. The field itself is never marked — nothing is marked wrong anywhere in the picker — so the box looks identical typed or not. What a reader can actually see is the picker: a variable's value could rail no candidate at all before, and rails them now. The reference tree shows the declared type beside every `var.*` row, where an expression-valued variable read `unknown`. `expectedAt` still said its argument is undefined for "a workflow variable, whose type is read from its value". Nothing reads a type from a value any more, so the sentence describes a field that no longer exists rather than the one it guards. --- docs/handoff.md | 17 ++++++++++------ .../packages/react/src/compounds/insertion.ts | 4 ++-- .../packages/react/src/layouts/Workflow.tsx | 20 ++++++++++++------- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/docs/handoff.md b/docs/handoff.md index 94a7401..9e24c67 100644 --- a/docs/handoff.md +++ b/docs/handoff.md @@ -145,12 +145,17 @@ subset already exists inside `scopeFor`, which computes it before appending upst it as `boardScope(doc, board, manifests, runContext)` and let `scopeFor` be that plus the Steps, so the two readers share one definition. -**A variable declares its type, so the value input carries a type marking like every other.** The -type is read from `t` rather than from the value beside it, because `core.set_var` writes the same -variable from a Step — so the literal in the document is only what it *starts* as, and a marking -inferred from it would be a claim about one moment in an execution (ADR-0013). The value input's -`expectedType` is therefore the declared type, and it is the one place a variable's value is a Slot: -everywhere else the variable is read rather than written. +**A variable declares its type**, read from `t` rather than from the value beside it, because +`core.set_var` writes the same variable from a Step — so the literal in the document is only what it +*starts* as, and a type inferred from it would be a claim about one moment in an execution +(ADR-0013). + +What that buys on this tab is the **completion list**, not a marking on the field: passing +`expectedType` is what lets the picker rail the candidate rows that fit, where a variable's value +input could rail none. Nothing is ever marked wrong — neutral covers "does not fit" and "cannot be +judged" alike — so the field itself looks the same either way. The declared type also shows beside +every `var.*` row in the reference tree, where an expression-valued variable previously read +`unknown`. **The type control is the one edit on the row that re-types downstream Expressions**, and the value box is not. That needs a test on both halves: retyping `threshold` from number to text changes the diff --git a/source/packages/react/src/compounds/insertion.ts b/source/packages/react/src/compounds/insertion.ts index 2baf8e9..7b00ff8 100644 --- a/source/packages/react/src/compounds/insertion.ts +++ b/source/packages/react/src/compounds/insertion.ts @@ -139,8 +139,8 @@ function tokenStart(written: string): number { * necessary is not a harmless conservatism — it is a green rail withheld from * a row that is exactly right, which is the only signal the rail carries. * - * Undefined when the field declares no type at all — a workflow variable, whose - * type is read *from* its value, so there is nothing to check it against. + * Undefined when the field declares no type at all, and then no candidate is + * ever railed: there is nothing to judge a row against. * * `[start, end)` is the range the insertion covers: the hole being edited, or * the caret. Whether what results is a whole-value Template is then one diff --git a/source/packages/react/src/layouts/Workflow.tsx b/source/packages/react/src/layouts/Workflow.tsx index 43f10d6..4beb951 100644 --- a/source/packages/react/src/layouts/Workflow.tsx +++ b/source/packages/react/src/layouts/Workflow.tsx @@ -578,12 +578,17 @@ const VARIABLE_TYPES = ['text', 'number', 'boolean', 'datetime', 'object', 'list /** * The workflow's variables: a key, a declared type and a Template, per row. * - * **The type is declared, not read off the value**, so the value box is a - * Template input with a type marking like every other. `core.set_var` writes the + * **The type is declared, not read off the value.** `core.set_var` writes the * same variable from a Step, which is what makes the literal in the document its - * FIRST value rather than its contract — a marking read off it would be a claim + * FIRST value rather than its contract — a type read off it would be a claim * about one moment in an execution (ADR-0013). The type control is therefore the - * one edit on this row that re-types every Expression reading the variable. + * one edit on this row that re-types every Expression reading the variable, and + * the value box is not. + * + * The declared type reaches the screen through the value box's completion list + * rather than through the box itself: it is what lets the picker rail the + * candidate rows that fit, where a variable's value could rail none. Nothing is + * ever marked wrong, so the field looks the same either way. * * **Renaming a key does not rewrite References.** `{{ var.old_name }}` goes * stale and the checker reports it, exactly as it does for a Step that was @@ -669,9 +674,10 @@ function Variables({ : String(variable.value) } scope={scope} - // The declared type, so the initial value is checked like every - // other Template. It is the one place a var's value is a Slot: - // everywhere else the variable is read rather than written. + // What the completion list judges a candidate against. It marks + // the rows that fit rather than the field itself — nothing here + // is ever marked wrong — so declaring the type is what lets the + // picker rail a row at all, where before it could rail none. expectedType={variable.t} onCommit={(next) => onValue(variable.key, next)} />