Stop validating fields the runtime copies verbatim (#104) - #480
Merged
Conversation
#471 established which parts of a step the runtime renders and wired that into dependency inference, so two inert strings stopped inventing a cycle. Template validation was still reading those same fields as if they resolved, and got both directions wrong: - id: a name: "{{ b.result }}" # "will be resolved at runtime" description: "{{ nosuch }}" # a hard error Nothing substitutes into `name`, so the braces reach the log verbatim and the warning told the reader the opposite of what happens. And a stray brace in prose *failed the pipeline* -- a false rejection of a document that runs correctly, which is the class removed by #465, #469 and #472 elsewhere. Inert fields now produce a warning that says what actually occurs, and nothing else. Renderable fields are untouched: an undefined name in `parameters`, `action` or `location` is still an error. The same defect exists one level up -- a pipeline's own `name:` was rejected too -- so `INERT_PIPELINE_FIELDS` sits alongside the step set. It is deliberately shorter than it first was: `version` is schema-constrained to `\d+\.\d+\.\d+`, so a template there is a real error and calling the field inert would describe it wrongly. The run in `test_a_pipeline_with_templates_in_prose_still_runs` is what caught that, and is the evidence for every field in both sets: a pipeline carrying an unresolvable reference in each prose field still executes and still writes the right contents. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jeremymanning
added a commit
that referenced
this pull request
Aug 20, 2026
…104) #480 applied `core/step_fields` to template validation and got the first half right -- a stray brace in a `description:` no longer rejects a pipeline that runs. It got the second half wrong by treating *every* unrendered field as prose, so all of these compiled with nothing but a wording note: metadata: goto: "{{ nosuch }}" # execution sent to a step named `{{ nosuch }}` priority: "{{ nosuch }}" # a priority of that text requires_model: "{{ nosuch }}" tool: "{{ nosuch }}" # a registry lookup for that text id: "{{ nosuch }}" Unrendered says what does *not* happen to a field, not what the field is for. Three cases, not one: * **prose** -- `name`, `description`, and metadata an author wrote for themselves. Nothing acts on it; the braces reach a log line and the pipeline runs. Still a warning, because the author will not get what they typed. * **structural** -- `id`, `tool`, `dependencies`, `depends_on`. These *name* things. A literal `{{ x }}` names nothing, so the pipeline is already broken and calling it valid says the opposite. Now an error. * **operational metadata** -- the keys the runtime reads. Not rendered, so control code receives the literal template text. Now an error. `OPERATIONAL_METADATA_KEYS` is evidence-backed rather than guessed: every key has a runtime read cited beside it, from `goto` at `orchestrator.py:1074` to `required_capabilities` at `core/control_system.py:130`. Keys the compiler *writes* -- `step_type`, `retry_count`, the loop bookkeeping -- are excluded, because an author never supplies them. Two boundaries the split needs beyond the three sets. A reserved key counts only at metadata's top level: `metadata.notes.priority` is somebody's data structure, not the key the runtime reads. And step recognition is now off entirely inside an inert subtree -- `metadata.steps` holding `for_each` and `while` was being walked as pipeline structure and reported as an ambiguous loop, from inside a subtree this module had just declared verbatim-copied. Separately, `create_template_issue` hardcoded `code="template_error"`, so every template finding arrived under one code and a consumer had to parse the message to tell an inert-field note from a loop-scope error. The validator's own `error_type` now flows through the compiler into the finding, which is what the structured payload exists for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jeremymanning
added a commit
that referenced
this pull request
Aug 20, 2026
…104) #480 applied `core/step_fields` to template validation and got the first half right -- a stray brace in a `description:` no longer rejects a pipeline that runs. It got the second half wrong by treating *every* unrendered field as prose, so all of these compiled with nothing but a wording note: metadata: goto: "{{ nosuch }}" # execution sent to a step named `{{ nosuch }}` priority: "{{ nosuch }}" # a priority of that text requires_model: "{{ nosuch }}" tool: "{{ nosuch }}" # a registry lookup for that text id: "{{ nosuch }}" Unrendered says what does *not* happen to a field, not what the field is for. Three cases, not one: * **prose** -- `name`, `description`, and metadata an author wrote for themselves. Nothing acts on it; the braces reach a log line and the pipeline runs. Still a warning, because the author will not get what they typed. * **structural** -- `id`, `tool`, `dependencies`, `depends_on`. These *name* things. A literal `{{ x }}` names nothing, so the pipeline is already broken and calling it valid says the opposite. Now an error. * **operational metadata** -- the keys the runtime reads. Not rendered, so control code receives the literal template text. Now an error. `OPERATIONAL_METADATA_KEYS` is evidence-backed rather than guessed: every key has a runtime read cited beside it, from `goto` at `orchestrator.py:1074` to `required_capabilities` at `core/control_system.py:130`. Keys the compiler *writes* -- `step_type`, `retry_count`, the loop bookkeeping -- are excluded, because an author never supplies them. Two boundaries the split needs beyond the three sets. A reserved key counts only at metadata's top level: `metadata.notes.priority` is somebody's data structure, not the key the runtime reads. And step recognition is now off entirely inside an inert subtree -- `metadata.steps` holding `for_each` and `while` was being walked as pipeline structure and reported as an ambiguous loop, from inside a subtree this module had just declared verbatim-copied. Separately, `create_template_issue` hardcoded `code="template_error"`, so every template finding arrived under one code and a consumer had to parse the message to tell an inert-field note from a loop-scope error. The validator's own `error_type` now flows through the compiler into the finding, which is what the structured payload exists for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows #479. Applies the
core/step_fieldscontract — which #471 established and wired into dependency inference — to template validation, which was still reading those fields as if they resolved.The defect, in both directions
A false promise. Nothing substitutes into
name; the braces reach the log verbatim, and the warning told the reader the opposite of what happens.A false rejection, and the more serious half: a stray brace in prose failed the pipeline. That is the class removed by #465, #469 and #472 elsewhere — validation rejecting a document that runs.
Inert fields now produce a warning saying what actually occurs:
Renderable fields are untouched: an undefined name in
parameters,actionorlocationis still an error. The warning names the inert field rather than the leaf key, sometadata.notereportsmetadatainstead of sending the reader looking for a rule about a key they invented.The same defect one level up
A pipeline's own
name:was rejected too, soINERT_PIPELINE_FIELDSsits beside the step set.It is deliberately shorter than my first version. I initially included
version, and the end-to-end run failed:versionis schema-constrained, so a template there is a real error and calling the field "inert prose" would describe it wrongly.idis excluded for the same reason. The set is nowname,description,metadata— exactly what a real run tolerates an unresolvable reference in.That is the point of
test_a_pipeline_with_templates_in_prose_still_runs: it executes a pipeline carrying an unresolvable reference in every field both sets claim is inert, and asserts the file still lands with the right contents. The declared sets are what execution proves, not what reading the source suggests — the same discipline as #479, and the reason theversionmistake did not ship.Verification
ruff check src/orchestrator --select E9,F63,F7,F82,F821,F823,F601,F811— cleanRebased onto main after #479 merged, and all three gates re-run on the rebased branch rather than assuming the earlier run still held.