JSON schema should be used to validate recipes at runtime

[kevinchannon]

Custom validation code is used at the moment. This is great for producing bespoke errors, but is a lot of code that could be deleted if JSON schema was used for validation. It would also help to keep the schema in line with actual behaviour.

[kevinchannon]

@claude based on the current codebase, what would be involved in tackling this issue?

I think it wouldn't be possible to guarantee that a tasktree file was schema-compliant directly, since some variable substitutions may have to be done before validating against the schema. Most variables should be OK, as long as they're wrapped in quotes. This must already be upheld, since we load the task as YAML.

Anyway, what could work and what will be harder?

[claude]

Claude finished @kevinchannon's task in 1m 22s —— View job

---

Tasks

• Read trigger comment and understand the request
• Review current validation code (parser.py, schema/)
• Analyze what schema validation could/couldn't cover
• Post analysis answer

Current state

• schema/tasktree-schema.json already exists and is reasonably comprehensive (covers imports, runners, interpreters, variables, tasks, naming patterns, etc.), but it is only used for editor tooling (VS Code/IntelliJ autocomplete, or the external check-jsonschema CLI). jsonschema is a dev-only dependency and is imported only in tests/unit/test_schema.py — nothing in src/tasktree/ validates against it at runtime.
• All runtime validation is hand-written: parser.py alone has ~128 raise ValueError(...) call sites doing structural checks (required fields, type checks, allowed-key checks, naming-pattern checks, etc.), spread across the whole file.
• 232 unit test assertions across the suite check the exact text of these error messages (assertRaisesRegex, str(cm.exception), etc.). That's the biggest hidden cost of migration — it's not just deleting validation code, it's a large surface of tests pinned to today's bespoke wording.

Your instinct about substitution is right, and splits the problem in two

What schema validation could straightforwardly replace (the "easy" half): pure structural checks that don't depend on runtime state — required fields (cmd on tasks), allowed top-level/nested keys, enum/pattern constraints (task/namespace naming regex, arg type values), and "must be a string/list/dict" checks. Since template placeholders ({{ var.x }}) only ever appear inside YAML string scalars — which, as you note, is already required for the YAML to parse at all — any schema field that's typed string keeps working whether or not it still contains unsubstituted templates. This class covers a large fraction of that 128, and could be swapped in incrementally: run jsonschema.validate() early in parse_recipe, then delete the manual checks it makes redundant one at a time (fits the project's "small commits" rule nicely — one manual check removed + its test updated per commit).

What gets harder — fields that are typed but may hold a template (e.g. min/max on numeric args, ports, docker build args values, anything declared integer/boolean/number in the schema). A literal min: 5 is fine, but min: "{{ arg.floor }}" is a string until substitution runs, so the schema would need loosening to anyOf: [{type: integer}, {type: string}] on every field that could plausibly carry a template — which erodes exactly the type-checking value schema validation was supposed to add. The alternative (validate once loosely pre-substitution, once strictly post-substitution) is more correct but is real added complexity, and post-substitution validation only fires per-task-invocation rather than at recipe-load time, so you lose "see all errors up front."

What schema validation structurally cannot do at all, and would have to stay as bespoke Python regardless:

• Circular reference/import detection (CircularImportError, variable-cycle detection) — inherently graph-based, not a per-node schema constraint.
• Cross-reference checks: dependency task names existing (possibly in another imported file), runner names referenced by tasks existing, no dots in runner names for namespacing.
• Order/lifecycle-dependent rules: pin_runner: true requires runner, but pinned-runner resolution is deliberately lazy (checked at invocation, not parse time) per CLAUDE.md — this is a runtime-state check, not a static shape check.
• Anything resolved through the two-tier runner factory (runner_from_config → containerised_runner_from_config) — the branch taken depends on type/engine combined with which fields are present, which JSON Schema can partially express via if/then but gets unwieldy fast for a hierarchy this polymorphic.

Net assessment

This is a real but incrementally tractable refactor, not an all-or-nothing rewrite:

1. Start by wiring jsonschema into the runtime path (promote from dev extra) purely as an additive first-pass check, without removing any existing manual validation yet.
2. Go field-by-field replacing manual "type/required/enum" checks with schema coverage, updating the ~232 message-dependent tests as each one is retired (each one being small commit).
3. Decide a policy for typed-but-templatable fields (loosen to accept strings, or split pre/post-substitution validation) — this needs a decision before step 2 touches those fields, since it changes the target schema shape.
4. Leave cross-reference/graph/lifecycle checks (circular imports, dependency existence, pinned-runner lazy resolution) as hand-written code permanently — schema was never going to own those.

The main risk isn't feasibility, it's that steps 2–3 touch a lot of surface area for "small, incremental commit" discipline to hold — I'd suggest tackling one config section at a time (e.g. runners first, since it's already isolated per the class hierarchy in CLAUDE.md) rather than attempting it as one PR.

[kevinchannon]

@claude I feel like a sensible flow would be something like:

1. Read in the tasktree file
2. work out which variables, interpreters, runners and tasks are "reachable" from the user's specified task
3. Resolve the necessary variable values (from the variables section) <--- Some validation of variables will need to happen here
4. Apply the Jinja2 template with the various variable/arg values. <--- Some validation of template substitutions will happen here
5. Validate the final, rendered YAML with only the necessary tasks/runners/interpreters in it<--- JSON schema used here
6. Execute the tasks

Does that seem possible, given the current code and findings above?

[claude]

Claude finished @kevinchannon's task in 3m 4s —— View job

---

Tasks

• Read trigger comment and understand the request
• Trace current execution order in executor.py/parser.py (parse → substitution → dependency resolution → execution)
• Check whether task/runner/variable "reachability" is currently computed anywhere (graph.py, parser.py)
• Assess each step of the proposed flow against current architecture
• Post analysis answer

Short answer

Yes, it's possible — and steps 1–3 are already closer to this shape than the earlier analysis assumed. The gaps are in step 4 (which isn't currently one unified operation) and step 5 (which doesn't exist as a concept at all yet).

Where the current code already matches your flow

• Step 1 (read file) — parser.parse_recipe() (parser.py:2594).
• Step 2 (reachability) — partially exists already. collect_reachable_tasks() (parser.py:2394) does a BFS over deps from the root task, and is used specifically to scope which variables get evaluated. It does not currently prune runners/interpreters — all of those in the file (and its imports) are always fully parsed into objects regardless of which task you asked for. Extending the same BFS to also narrow runners/interpreters looks tractable, since runner lookups are already by-name.
• Step 3 (resolve variables) — already happens exactly like this: Recipe.evaluate_variables(root_task) (parser.py:505) resolves only the reachable subset of variables: (including env/eval/read sourced ones) right after parsing, before the dependency graph is built.

Where it diverges from the current code

• Step 4 ("apply the template") isn't one step today — it's three separate passes at three different times: (a) var.* at parse time (folds into step 3 above), (b) dep.*.outputs.*/self.* substituted in topological order by graph.py (resolve_dependency_output_references/resolve_self_references, graph.py:328/437) before execution starts, mutating Task fields in place, and (c) arg.*/env.*/tt.* substituted per-field at actual execution time inside Executor._run_task (executor.py:1548–1595), as transient strings that are never written back anywhere.
• There's also an inconsistency worth flagging on its own: only cmd/working_dir go through the newer Jinja2 renderer (rendering.py, wired via executor.py:1494–1529). Docker-runner fields — volumes, ports, env_vars, container working_dir — still go through an older regex substitution path (executor.py:1304–1341) that doesn't even support arg.*. Unifying that onto Jinja2 is probably a prerequisite for step 4 to become one clean, reusable "render" call you can point schema validation at.
• Step 5 (validate the final rendered YAML) is genuinely new — there is no point today where a fully-substituted task/runner/interpreter set is reconstituted into a single dict/document. You'd need to add a projection step, after (b)+(c) render for a given task in the execution order, that reassembles that task + its resolved Runner + Interpreter into a plain dict shaped like the schema's tasks:/runners:/interpreters: sections, then run jsonschema.validate() against it.
  • The runner class hierarchy (Runner/HostRunner/ContainerisedRunner/DockerRunner/NixRunner, parser.py:104–197) means that projection — or the schema itself — needs to branch per runner kind, since field sets differ structurally.
  • For the "typed-but-templated" fields flagged in the earlier analysis (min/max, docker ports, build args): since Jinja always renders to a string, feeding the rendered scalar back through yaml.safe_load() before validating would recover its native int/bool/float type. That avoids loosening the schema's types to anyOf[string, ...] everywhere, which was the main worry before.

Net effect on the earlier analysis

Because step 5 would naturally slot in per invoked task (using that task's actual bound args), schema validation would happen per-invocation rather than as a single static whole-file check — you'd still lose "see every schema error across the file up front," same conclusion as before, just now for a more concrete reason (most tasks' arg.* values simply don't exist until invocation). The cross-cutting checks called out previously (circular imports, dependency-name existence, pinned-runner's deliberately lazy resolution) still can't move into schema no matter where in this pipeline it runs.

Suggested incremental slicing, in commit-sized pieces: (1) unify Docker-runner field rendering onto rendering.py's Jinja engine, retiring the parallel regex path; (2) extend collect_reachable_tasks-style pruning to runners/interpreters (optional, independent cleanup); (3) add the render→dict projection for one runner kind at a time; (4) wire jsonschema.validate() in additively, without removing any hand-written check yet.

[kevinchannon]

@claude , some more things. Can you understand and assess the questions at the end, given the statements about what I want to happen.

What I want to happen

The tasktree file is read from disk first. This stage can produce errors if the file is malformed to the extent that it isn't even valid YAML.

After reading in the tasktree file, the first thing that should happen is to do all the imports; so we end up with all possible elements in the same tree (YAML). There is some implicit validation at this point, since finding a non-existent imported file is an error.

Then, understand the reachable tasks. Errors can occur here if à task that is reachable (and is therefore being encountered in the tree traversal) has a non-existent dependency. This stage is the first pruning stage and drives all subsequent pruning. Now prune away all the unreachable tasks.

Once the only tasks that exist are the reachable ones, the runners and interpreters can be understood. Só, we can prune all the unreachable runners and interpreters. This point could also generate errors, if a reachable task refers to a non-existent runner or interpreter.

With the tasks, runners and interpreters pruned, the schema check can be applied to the resulting template.

If the schema check passes, the required variables can be collected and evaluated. Errors can occur at this stage in the evaluation of the variables themselves.

After variable evaluation, their runners and Jinja config can be prepared and then applied to the interpreters and runners sections of the template (that is, the pruned YAML object).

Now, the runners and interpreters are completely rendered, the tasks can be rendered and then executed one-by-one. They cannot all be rendered simultaneously because the outputs of one task may flow into another as inputs.

The question

Are there any obvious issues with this strategy? If so, what are they? How serious?

What is required to refactor the code into a scheme like this, só that the JSON schema is meaningfully used to validate the incoming tasktree file?