Use JSON schema to validate incoming tasks

[kevinchannon]

This PR is being opened prematurely to allow assessment of the state of CI tests against this work.

Eventually thë whole feature will be present and we can merge it.

[claude]

Review

This PR is explicitly a WIP checkpoint (slices 0–3 of the schema-validation-pipeline plan, issue #43), so I focused the review on the code that's actually landed rather than on the plan's not-yet-built slices.

Summary of the change

• Unifies runner/interpreter field substitution onto the Jinja renderer (Executor._render_runner_field), deleting the old regex-based _substitute_builtin/_substitute_env wrappers in executor.py.
• Adds a generic, regex-based reference walker (src/tasktree/template_refs.py: collect_template_refs, expand_variable_refs) for discovering {{ prefix.name }} references across a raw recipe subtree.
• Adds check_runner_template_refs (parser.py), called from build_recipe_runner and _parse_inline_interpreter, which rejects arg.*/dep.*/self.* and per-task tt.* builtins (task_name, working_dir, timestamp, timestamp_unix) inside runner/interpreter definitions at parse time, since those definitions are shared across tasks and rendered once.
• task_config.build_runner_config assembles the restricted (env + tt only) rendering context for runner fields.

Code quality / best practices

• Nice adherence to the project's stated philosophy: small commits, tests alongside each increment, and a documented "reference gate" comparing behaviour against v1.3.2 with an explicit expected-divergences list. The plan doc is kept in sync with what actually landed.
• check_runner_template_refs and build_runner_config are small, well-named, single-purpose functions with docstrings that state pre/post-conditions — matches the repo's "small named functions over comments" guidance.
• Minor redundancy: build_recipe_runner calls check_runner_template_refs(config, ...) on the entire raw runner config (which already includes a nested interpreter: subtree), and then parse_interpreter_spec → _parse_inline_interpreter calls it again on just the interpreter subtree. Harmless (recipes are small, parsed once), but worth a one-line comment noting it's intentional double coverage, or a follow-up to avoid re-walking the same nodes.
• The plan doc (docs/plans/schema-validation-pipeline.md) keeps each "✅ done" slice's completion summary directly above the original (now historical) planning prose for that slice ("Original scope follows."). Intentional per the doc's own convention, but it reads as duplicated content on a first pass — worth a note for future readers unfamiliar with the convention.
• The plan records that "Kevin has granted per-increment local commits for this work (supersedes CLAUDE.md's never-commit rule)" — good that the exception is explicit and documented in the plan rather than silently diverging from CLAUDE.md.

Potential bugs / issues

• template_refs._REFERENCE's name pattern ([A-Za-z_][A-Za-z0-9_-]*(?:\.[A-Za-z_][A-Za-z0-9_-]*)*) requires each dotted segment to start with a letter/underscore, so a positional reference like {{ self.inputs.0 }} only captures self.inputs, dropping the numeric index. This doesn't affect the current use (check_runner_template_refs only needs to know a forbidden prefix was used, not the exact dotted path), but it's a latent under-matching gap if collect_template_refs is later reused for hashing/pruning (per the plan's slice 5 notes), where the full dotted path matters more. Worth a test/fix before that reuse happens.
• Defense-in-depth is solid: even if the regex walker under-matches (e.g. bracket-style Jinja access like {{ arg['x'] }}), the restricted build_runner_config context (no arg/dep/self keys) plus StrictUndefined means such a reference would still fail at render time — just with a less specific error message than the parse-time check produces. Not a correctness bug, just a slightly worse error message in that edge case.

Performance

• No concerns. The added regex walking runs once at parse time over small YAML trees; no hot-path impact.

Security

• No concerns. This is pure validation/rendering logic operating on trusted recipe YAML — no new subprocess/eval/network surface, and the regexes have no catastrophic-backtracking shape (no nested quantifiers).

Test coverage

• Strong coverage for what's landed: tests/unit/test_template_refs.py covers the walker (all prefixes, nested dicts/lists/keys, Jinja conditionals, cycles/fixpoint closure); tests/unit/test_runner_template_restriction.py covers end-to-end parse-time rejection/acceptance across runners, inline task runners, and interpreters, and is deliberately kept import-minimal so it doubles as the v1.3.2 reference-gate fixture; tests/unit/test_executor.py adds parity + rejection tests per runner field (volumes, env_vars, ports, working_dir, run args, preamble).
• One small regression in assertion strength: tests/integration/test_builtin_variables.py's TASK_NAME_VAR → USER_NAME_VAR swap changed the assertion from checking an exact expected value ("docker-test") to just assertNotIn("{{", ...). That's a reasonable trade-off since tt.user_name is environment-dependent, but it's worth knowing the test now only proves "something got substituted," not "the right thing got substituted."

Action items

• Consider avoiding the double check_runner_template_refs walk (once over the whole runner config in build_recipe_runner, again over just the interpreter subtree in _parse_inline_interpreter) — e.g. by having build_recipe_runner skip the interpreter key before checking, or documenting why the double-check is intentional.
• Add a test (or fix) for positional self.inputs.N/self.outputs.N references in collect_template_refs, since the current regex drops the trailing numeric segment — flag this now so it doesn't silently bite slice 5's planned reuse of the walker for reachability/pruning.
• Optional: strengthen the updated USER_NAME_VAR integration test (e.g. mock/patch user_name to a known value) so it verifies the correct value was substituted, not just that substitution happened.

[kevinchannon]

Review action items addressed in d2b44e4:

1. Double check_runner_template_refs walk — kept, now documented as intentional at the call site in build_recipe_runner. The outer walk covers the whole raw config; the inner check in _parse_inline_interpreter is what protects interpreters defined outside any runner (the interpreters: section and task-level overrides). Skipping the interpreter key in the outer walk would couple the function to that call graph for no measurable saving.

2. Numeric segments dropped from positional refs — fixed. _REFERENCE continuation segments may now start with digits, so {{ self.inputs.0 }} extracts inputs.0 (and {{ dep.build.outputs.0 }} keeps its index). Tests added for both. As the review noted, this didn't affect the current prefix-only consumer, but it closes the under-matching gap before slice 5/7 reuse the walker for pruning/hashing.

3. USER_NAME_VAR assertion strength — strengthened. The test now pins os.getlogin to a known value and asserts the exact substituted string, restoring the pre-change assertion strength without depending on the environment. Reference gate: the updated test passes against v1.3.2 (parity).

Full pyramid run after the changes: unit+integration 1647 passed / 4 conditional skips; e2e 71 passed / 5 conditional skips.

[claude]

Review of PR #212 — "Use JSON schema to validate incoming tasks" (slices 0-3, WIP)

Thanks for the clear per-slice commit history and the self-documenting plan doc - it made this easy to follow even without the originating conversation. This is explicitly an in-progress WIP PR per the description, so I focused on the substance of slices 1-3 that has landed rather than the overall roadmap.

Summary of what's in this PR

• Slice 1: runner/interpreter field substitution (volumes, ports, env_vars, working_dir, dockerfile, context, docker build/run args, interpreter cmd/preamble) is unified onto the Jinja rendering.render() path via a new Executor._render_runner_field, replacing the old regex-based _substitute_builtin/_substitute_env helpers (deleted).
• Slice 2: a new, generic collect_template_refs/expand_variable_refs walker in template_refs.py that finds {{ prefix.name }} references anywhere in a raw recipe subtree.
• Slice 3: check_runner_template_refs (parser.py) uses the slice-2 walker to reject arg.*/dep.*/self.* and per-task tt.* builtins (task_name, working_dir, timestamp, timestamp_unix) inside runner/interpreter definitions, since runners are shared across tasks and rendered once.

Correctness issue: the "runners can't see per-task values" guarantee doesn't actually hold everywhere

The core safety property this slice is building towards is that runner/interpreter fields must not depend on per-task state, because a runner is shared across tasks. Two gaps undermine that today:

1. Machine-config runners silently pick up per-task tt.* values instead of failing. check_runner_template_refs is only wired into the recipe path (build_recipe_runner in parser.py:2096, and _parse_inline_interpreter for interpreter definitions). config.py's default-runner loader (~line 298) builds machine-config runners straight through runner_from_config, which never calls check_runner_template_refs. The plan doc (docs/plans/schema-validation-pipeline.md, slice 3 notes) states this is fine because "their forbidden refs still fail at render via slice 1's strict Jinja" - but that's only true for arg/dep/self (absent from the render context entirely, so Jinja's StrictUndefined raises). It is not true for tt.*: task_config.build_runner_config (task_config.py:173-176) does "tt": dict(builtins or {}), forwarding the entire builtins dict - including task_name, working_dir, timestamp, timestamp_unix - unfiltered into the Jinja context (confirmed via Executor._collect_builtin_variables, executor.py:239-297, which always includes those four per-task keys, and _run_task_in_docker, executor.py:1399, which passes that dict straight into _substitute_builtin_in_runner/_render_runner_field). So a machine-config runner with e.g. env_vars: {TASK: "{{ tt.task_name }}"} will render successfully - silently varying its value depending on which task happens to be running - instead of erroring, which is exactly the class of bug this feature is meant to close off. There's no test exercising this path (grep -rn "runner_from_config" tests/ turns up nothing checking tt.task_name/per-task rejection there).
2. Recipe runners can bypass the check via variable indirection. check_runner_template_refs calls collect_template_refs(subtree) directly on the raw config - it never calls expand_variable_refs (the transitive-closure helper slice 2 built specifically for this kind of chasing). So runners: { docker: { env_vars: { X: "{{ var.mount }}" } } } with variables: { mount: "{{ arg.thing }}" } passes the parse-time check (it only sees var.mount), and only fails later at render time with a generic "Undefined variable: 'arg' is undefined" Jinja error instead of the intended, more actionable message pointing at the runner/interpreter definition. Minor by comparison to (1) since it does still fail, just with a worse error and later than intended.

Suggest: for (1), either call check_runner_template_refs from config.py's runner-building path too, or (better, since it closes the hole for both paths at once) filter build_runner_config's tt dict down to the allowed four global names always, so silently-wrong values become fail-fast even for uncovered call sites. For (2), run the check against expand_variable_refs(collect_template_refs(subtree), raw_variables) rather than the raw refs, so indirection through a variable doesn't hide a forbidden namespace.

Smaller notes

• docs/plans/schema-validation-pipeline.md's slice-3 completion note asserting per-task tt.* refs "fail at render" for machine-config runners should be corrected/removed once (1) above is addressed - right now it documents behaviour that isn't actually true, which could mislead whoever picks up slice 4+.
• Good instinct writing tests/unit/test_runner_template_restriction.py self-contained (only importing parse_recipe) for the v1.3.2 reference-gate comparisons - matches the plan's own stated gate practicalities.
• Test coverage for the new Jinja-based runner-field rendering (slice 1) and the walker (slice 2) both look solid and well-targeted at the field list actually in play (including the wider list found in slice 0's extra finding: dockerfile/context/build+run args/interpreter cmd+preamble).

Performance / security

No concerns - collect_template_refs's regex is non-greedy and bounded per {{ ... }} block, no ReDoS risk from the patterns used. No injection concerns; this is parse-time validation logic only.

---

Action items

• Close the machine-config runner gap: either call check_runner_template_refs from config.py's runner-loading path, or make build_runner_config's tt namespace always restrict to the four global builtins (project_root, recipe_dir, user_home, user_name) regardless of caller, so per-task values can never silently leak into a shared runner's rendered fields.
• Add a regression test that a machine-config runner referencing {{ tt.task_name }} (or another per-task builtin) either fails to parse/load or fails to render - currently nothing catches this.
• Have check_runner_template_refs use expand_variable_refs (not just collect_template_refs) so a forbidden namespace referenced indirectly through a {{ var.name }} in a runner/interpreter field is caught at parse time with the intended error message, not a generic Jinja "undefined" error at render time.
• Update the "forbidden refs still fail at render via slice 1's strict Jinja" note in docs/plans/schema-validation-pipeline.md once the above is fixed, since it currently overstates what's actually guaranteed for tt.* in machine-config runners.

[claude]

Review of #212 — Use JSON schema to validate incoming tasks

Reviewed via gh pr diff 212 against main (24 files, +4489/-937). Per the PR description and docs/plans/schema-validation-pipeline.md, this is explicitly a work-in-progress checkpoint (slices 0–5 + slice 6 part 1 of an 8-slice plan tracked in #43), opened early to see CI results rather than for merge. Reviewing it as such — mostly architectural soundness and risk-spotting rather than "is this done."

Overview

The PR replaces object-based recipe parsing (_parse_file/_parse_file_with_env building Task/Runner objects while recursing through imports) with a raw-dict merge phase (src/tasktree/raw_merge.py) that resolves imports, namespacing, run_in blankets, and variable-reference rewriting entirely on plain dicts before any object is constructed. Objects are then built from the merged tree in one pass. This is the necessary precondition for real JSON-schema validation (you can only run jsonschema.validate() against a dict, not a Task), and it's a big, well-telegraphed refactor — the tracking doc's slice-by-slice log is genuinely helpful for review.

New/notable pieces:

• src/tasktree/raw_merge.py — the merge engine (590 lines, new).
• src/tasktree/template_refs.py — a single generic {{ prefix.name }} reference walker (collect_template_refs) shared by variable-reachability, runner-field restriction, and (per the plan) future task hashing — replaces several bespoke enumerated-field-list scanners in parser.py.
• Task-independent runner rendering (Executor._render_runner_field / task_config.build_runner_config) — runner/interpreter fields now render through the same Jinja path as task fields, restricted to env.*/tt.*.
• Name-aware state pruning (state.py: TaskState.task_name) so pruning can distinguish "task deleted" from "task just wasn't part of this run" — this also fixes a real pre-existing v1.3.2 bug (state thrash on unrelated targeted runs) called out honestly in the expected-divergences list.
• Task/runner/interpreter pruning to the reachable subtree on invocation, so parse-time defects in unreached parts of a recipe no longer block a run.
• Schema fix: variables schema had integer and number as separate oneOf branches, so every int matched both and was rejected (e.g. port: 8080) — good catch, and it's backed by a 355-recipe fixture-corpus regression test (TestFixtureCorpus) that pins "the schema must never be stricter than the parser." That's a strong regression net for slice 6 parts 2–3.

Code quality / design

• The class-based Runner hierarchy and polymorphic dispatch described in CLAUDE.md are respected — no new type-string branching introduced.
• raw_merge.py is a clean, self-contained module with docstrings that explain why, not just what (e.g. why imported tasks merge before local, why namespacing walks values but not keys). Matches the repo's "comment on WHY" convention well.
• template_refs.py is a good consolidation — one regex-based walker instead of N enumerated per-field scanners means new fields can't silently fall outside variable/runner-restriction scanning again.
• Old dead code (_parse_file, _parse_file_with_env, ParsedFileResult, the five _rewrite_*_variable_references helpers, collect_reachable_tasks/collect_reachable_variables) is fully deleted rather than left behind — no half-migrated state.
• Deliberate "over-match, never under-match" bias is stated explicitly and consistently applied across the walker, variable reachability, and pruning — a sound default for a scanner that feeds correctness-sensitive decisions (missing a ref breaks a run; an extra one is just wasted work).

Potential issues

1. Silent drop of imported runners/variables/interpreters on a malformed local section (raw_merge.py, the three elif merged_*: if local_* is None: blocks). If a file's own runners:/variables:/interpreters: key is present but not a dict (and not None) — e.g. a typo like runners: docker instead of runners: {docker: {...}} — the merge neither raises nor keeps it, and any imported runners/variables/interpreters for that file are silently dropped rather than surfaced. Once part 3 wires jsonschema.validate() this becomes moot (schema will reject the malformed section), but until then it's a quiet failure mode worth a test either way (or a comment noting it's intentionally deferred to the schema).
2. _apply_runner_transforms namespaces a string runner: value when present, and applies the blanket only when both runner and pin_runner are absent — worth double-checking the interaction with pin_runner: true + no runner at all: a pinned task with no runner should presumably still error (per the "pinned tasks must have runner specified" rule in CLAUDE.md). Confirm that validation still fires post-merge — didn't see it explicitly exercised in this diff; may already live elsewhere and be untouched, but worth a regression test given how much of the surrounding logic just moved.
3. The reference-gate methodology (comparing against a pinned v1.3.2 worktree) is a good idea, but it lives entirely in a markdown doc with manual git worktree steps ("recreated at 36de66d... if it goes missing again"). That's a footgun for anyone other than the author reproducing verdicts later — worth eventually turning into a script or at least pinning the commit/tag in a constant rather than prose.
4. The plan doc notes slice 6 parts 2–3 (merged-tree schema generator, wiring jsonschema.validate()) are not yet in this diff — so the schema itself still isn't enforced at runtime. Worth being explicit about that in the PR description (currently just says "premature... to allow assessment of CI") so reviewers don't go looking for the validation call.

Test coverage

Strong — roughly 190 new/changed test functions across unit (test_raw_merge.py with a parity-vs-object-path class, test_template_refs.py, test_runner_merge_cutover.py, test_task_merge_cutover.py, test_variable_reachability.py, test_runner_template_restriction.py, test_schema.py) and integration (test_state_pruning.py, test_unreachable_task_tolerance.py, test_builtin_variables.py). The fixture-corpus schema test and the "reference gate" divergence tracking are both good practices worth keeping as the plan continues — they turn "did I preserve v1.3.2 behavior except where documented" into an automatable check rather than a manual review burden.

Security

No concerns — this is parse-time/pure-dict logic, no new subprocess, network, or filesystem-write surface. _load_yaml correctly still uses yaml.safe_load.

---

Actions arising

• Decide whether the silent-drop case for a malformed runners:/variables:/interpreters: section combined with imports (raw_merge.py, the elif merged_*: if local_* is None branches) needs a test/guard now, or is explicitly deferred to slice 6 part 3's schema wiring — add a one-line note either way.
• Confirm "pinned task must specify runner" validation still runs correctly against the merged tree and add a regression test if it isn't already covered post-cutover.
• Consider scripting the reference-gate worktree setup (~/repos/tasktree-ref at a pinned commit) instead of leaving it as manual steps in the plan doc, so future divergence checks are reproducible without tribal knowledge.
• When slice 6 parts 2–3 land (merged-tree schema generator + wiring jsonschema.validate()), update the PR description/title to reflect that runtime enforcement is now actually active, since right now the schema file changes are unused by tt at runtime.
• Continue the "expected divergences" tracking in the plan doc for the new prospective divergences already called out (non-string ports/volumes/env_vars, desc: 42, templated private/pin_runner/task_output, task_output not converted to TaskOutputTypes at parse) so they don't get lost before part 3 lands.

[claude]

Review summary

This PR (marked by the author as opened prematurely, purely to check CI) is a large, well-executed refactor that replaces the ad-hoc regex-based recipe parsing/substitution machinery with:

1. A unified raw-dict import merge phase (raw_merge.py) that applies namespacing, run_in blanket overrides and pinned-runner rewriting as dict transforms before any Task/Runner object is built.
2. A shared template reference walker (template_refs.py) used for pruning, hashing and the new runner variable-class restriction, so those three consumers can't disagree about what a subtree references.
3. JSON Schema validation of the merged tree, derived programmatically from the authored file schema (recipe_schema.py) so the two schemas can't drift apart, with a message layer that turns jsonschema's raw errors into recipe-relative, human-readable text.
4. Unifying runner/interpreter field substitution onto the Jinja renderer (_render_runner_field), deleting the old _substitute_builtin/_substitute_env regex helpers, plus a new parse-time check (check_runner_template_refs) that rejects per-task namespaces (arg.*, dep.*, self.*) in runner/interpreter definitions.
5. Name-aware state pruning (TaskState.task_name) so a deleted task's stale cache entry can be distinguished from a defined-but-not-invoked task's entry.

Overall this is a real quality improvement: ~600 lines of duplicated/branchy parsing logic in parser.py are deleted and replaced by focused, well-documented, single-purpose modules. The docstrings consistently explain why (not just what), matching the project's Clean Code preference for named functions over comments. The 67 commits (per the linked plan doc, docs/plans/schema-validation-pipeline.md) show a genuinely incremental, test-as-you-go slice-by-slice history with a "reference gate" (diffing behaviour against v1.3.2) — exactly the workflow described in CLAUDE.md.

Strengths

• DRY across consumers: collect_template_refs is shared by pruning, hashing, and the runner-restriction check, avoiding three independent regex implementations that could silently disagree.
• Schema derivation instead of duplication: merged_tree_schema() transforms the authored file schema rather than hand-maintaining a second schema, with an explicit ValueError if a new name pattern isn't accounted for — a good guard against silent drift.
• Error message quality: test_schema_errors.py specifically asserts messages stay short, name the recipe file/location in recipe terms, and never leak jsonschema internals (additionalProperties, should not be valid under..., etc.). This is exactly the kind of behavioural test the CLAUDE.md testing philosophy asks for.
• Correct handling of tricky cases: diamond imports (non-circular), mutual/self circular imports, blanket run_in vs. pinned-runner precedence, and the new check_runner_template_refs restriction all have dedicated tests, including a "must not raise" acceptance test alongside the rejection tests.
• Immutable-style recursion fix: _merge_file's import_stack = import_stack + [file_path] avoids the old _parse_file's mutate-then-.copy()-before-recursing pattern, which is a nice correctness simplification.

Potential issue: variable-reference namespacing misses non-trivial Jinja expressions

raw_merge._namespace_var_refs (built on VAR_REFERENCE_REWRITE_PATTERN) only rewrites a {{ var.X }} reference when it is the entire content of the template block:

VAR_REFERENCE_REWRITE_PATTERN = re.compile(r"(\{\{\s*var\.)([^\s}]+)(\s*}})")

Since the underlying renderer (rendering.py) is a full Jinja2 Environment, and template_refs.py's own extractor is explicitly designed to find refs anywhere inside a {{ ... }} block (its docstring/tests even cover {{ var.a if flag else var.b }}), a variable reference used with a filter or inside a larger expression in an imported file will not get namespaced during the merge, e.g.:

# lib.yaml, imported "as: lib"
variables:
  greeting: hello
tasks:
  hi:
    cmd: "echo {{ var.greeting | upper }}"   # not rewritten to var.lib.greeting

After merge, the cmd string still reads var.greeting (unnamespaced), while variables only carries the namespaced key lib.greeting. Because rendering.py uses StrictUndefined, this fails at render time with an "undefined" error if no root-level greeting variable exists — or, worse, silently resolves to the wrong variable if a same-named variable happens to exist at another scope, since nothing else catches it (it's not caught by _schema_validate, check_runner_template_refs doesn't apply to task fields, and _collect_referenced_variable_names/collect_template_refs — which does find the un-namespaced ref — would only either evaluate a nonexistent/wrong-scope variable or leave it unevaluated).

This exact whole-block-only limitation already existed pre-PR (the module's own comment notes it's "deliberately broader than the old per-field rewrite in parser.py", i.e. still narrower than the block-scanning approach template_refs.py demonstrates is achievable), so it isn't a regression this PR introduces — but the PR's own test suite (template_refs.py's tests, the plan doc's slice 2 notes) shows filtered/conditional Jinja expressions are an anticipated, tested pattern elsewhere in the same templating system, so the gap is more likely to be hit as this work lands. I couldn't find a test in test_raw_merge.py covering a var reference with a filter/expression across an import boundary, and no mention of this as an accepted divergence in docs/plans/schema-validation-pipeline.md.

Suggestion: either (a) make _namespace_var_refs scan for var.* references anywhere inside a {{ ... }} block (mirroring template_refs.py's block-then-reference two-step) so filters/expressions get rewritten too, or (b) if that's out of scope for this slice, add a test documenting the current limitation and consider raising a clear parse-time error when an imported file's var.* reference can't be safely namespaced (rather than failing silently or late at render time).

Minor notes

• raw_merge._merge_file re-parses and reprocesses an imported file's whole subtree once per import site, so a "diamond" import (the same file imported from two different places) does double the YAML-load + transform work. Not a correctness issue (there's a test confirming diamond imports merge correctly), just a possible future perf/readability consideration if diamond imports become common in large recipes.
• docs/plans/schema-validation-pipeline.md is a large, detailed planning/progress doc checked into the repo. Worth deciding whether it stays long-term in docs/ post-merge or gets trimmed once the migration is complete, so docs/ doesn't accumulate stale planning artifacts.
• Since this PR is explicitly "opened prematurely" to check CI, it's worth confirming the full 67-commit history is intended to land as-is on main vs. being squash-merged — the CLAUDE.md incremental-commit philosophy seems well satisfied by the per-slice commit structure already, so a regular merge (preserving history) may be preferable to squashing all of this into one commit.

Test coverage

Coverage looks thorough: new unit tests for raw_merge (imports, namespacing, circular/diamond detection), template_refs, the runner-template restriction, schema error messages, and updated integration/e2e tests for nested invocations, recursion detection, state pruning, and unreachable-task tolerance. No unconditional test skips spotted, matching the project's testing philosophy.

---

Action items

• Decide whether _namespace_var_refs/VAR_REFERENCE_REWRITE_PATTERN should be extended to rewrite var.* references embedded in larger Jinja expressions (filters, conditionals, concatenation) across import boundaries, not just whole-block references — and add a regression test for a filtered/expression var reference in an imported file.
• If extending the rewrite is out of scope for this slice, document the current whole-block-only limitation explicitly (e.g. in the plan doc's divergences list) and consider a clear parse-time error instead of a silent/late failure when it's hit.
• Confirm the intended merge strategy (regular merge preserving the 67-commit slice history vs. squash) once this PR is out of "premature" status.
• Decide the long-term fate of docs/plans/schema-validation-pipeline.md (keep as historical record vs. trim once the migration lands).