Skip to content

Stop unresolved templates before they reach a tool (#153) - #439

Merged
jeremymanning merged 1 commit into
mainfrom
fix/unresolved-templates-153
Aug 1, 2026
Merged

Stop unresolved templates before they reach a tool (#153)#439
jeremymanning merged 1 commit into
mainfrom
fix/unresolved-templates-153

Conversation

@jeremymanning

Copy link
Copy Markdown
Member

Closes #153.

Measured before

$ orchestrator run bad_template.yaml
"success": true, "error": null

$ cat out/bad.txt
value = {{ nonexistent_step.result.field }}

Literal Jinja source on disk, in a file a user would read as output, from a run that exited reporting no error. validate refused the same document — so this is the mirror image of #241, with the validator right and the executor wrong.

Measured after

UnresolvedTemplateError - Parameter 'content' of tool 'filesystem' still contains
unresolved template references: '{{ nonexistent_step.result.field }}'.
The step was stopped before it could act on them.

PipelineExecutionError: Pipeline execution failed: Task 'write_it' failed
$ cat out/bad.txt
(no file written)

Exit 1, no artifact, the reference named.

Where the fix goes, and why not where the bug is

TemplateManager catches UndefinedError, logs a warning, and returns the original template text (template_manager.py:516). That fallback is deliberate and has to stay — resolution runs in several passes, and a reference that cannot resolve yet may resolve later. Re-raising there would break legitimate deferred resolution.

What was missing is a point where the fallback stops being acceptable. That is resolve_before_tool_execution, which already promised "tool parameters with all templates resolved" and is the last place the promise can be kept: past it the value is written to a file or sent to a model. It is also the single funnel — Tool.execute routes every tool through it (tools/base.py:40), so this is one edit rather than one per output-writing tool.

The check now enforces what the docstring already claimed.

Survival, not presence

A first version compared only the rendered output and flagged any {{ }} it found. That rejected escaped Jinja: {{ '{{' }} renders to a literal {{, so writing documentation about templates became impossible. Its own test caught it.

The rule is now: flag a marker only if it appeared in the input and came back byte-identical. A marker present in the output but not the input is content — escaped delimiters, or a step result that legitimately contains template-looking text (a scraped page, a code sample). Both cases have tests.

One bad reference takes the whole string down

StrictUndefined aborts the entire render, so in "{{ here }} but not {{ missing }}" the resolvable {{ here }} does not survive either. Both markers are reported.

I initially asserted only {{ missing }} would be reported, and the test failed — the code was right and my expectation was wrong. Reporting just the undefined reference would imply the others had been substituted. This behaviour is now pinned and documented, because a user hitting it would otherwise find it baffling.

Coverage

tests/test_unresolved_templates.py — 21 tests:

  • end to end through the CLI: non-zero exit, no artifact written, the reference named in the output
  • a resolvable template still passes (no false positives)
  • the error names the parameter, the tool, and the surviving markers
  • {% if %} blocks caught, not just {{ }}
  • references buried in nested lists/dicts caught
  • _-prefixed plumbing parameters not inspected
  • escaped delimiters and template-looking step results both pass
  • the marker scanner, parametrised over strings, dicts, dict keys, nested lists, tuples, non-strings, and multi-line blocks

Numbers

before after
Blocking suite 435 passed 456 passed, 13 skipped, 0 failed, 0 xfailed
Collection 3230 3251, 0 warnings
uv lock --check / ruff on src / git diff --check clean

ADR 0001 now states this contract and the action registry from #241; neither was written down.

Note on scope

This closes the reported defect at the tool boundary. It does not make an unresolved reference a compile-time error — validate already rejects this pipeline, so the two now agree, but a reference that only becomes resolvable at runtime is still legal by design. Tightening that further is a schema decision, not a bug fix.

Measured before this change, on a pipeline whose `content` referenced a
step that does not exist:

    $ orchestrator run bad_template.yaml
    "success": true, "error": null
    $ cat out/bad.txt
    value = {{ nonexistent_step.result.field }}

The literal Jinja source on disk, in a file a user would read as output,
from a run that exited reporting no error. `validate` refused the same
document -- so this is the mirror image of #241, with the validator right
and the executor wrong.

Root cause: TemplateManager catches UndefinedError, logs a warning and
returns the original template text. That fallback is deliberate and has
to stay, because resolution runs in several passes and a reference that
cannot resolve yet may resolve later. What was missing is a point where
the fallback stops being acceptable.

That point is resolve_before_tool_execution, which already promised
"tool parameters with all templates resolved" and is the last place the
promise can be kept -- past it the value is written to a file or sent to
a model. It is also the single funnel: Tool.execute routes every tool
through it. The check now enforces what the docstring claimed.

The test is *survival*, not presence. A marker in the output that was not
in the input is content: Jinja's own escape `{{ '{{' }}` renders to a
literal `{{`, and a step result may legitimately contain text that looks
like a template. A first version compared only the output and rejected
escaped Jinja -- caught by its own test, which is why the rule is now
"appeared in the input and came back byte-identical".

Nested containers are walked, since a parameter is routinely a list of
paths or a dict of fields and a reference buried in one reaches a tool
just as readily. `_`-prefixed parameters are skipped: those are plumbing
the control systems attach on the way to a tool, not authored values.

Rendering turns out to be all-or-nothing per string -- StrictUndefined
aborts the whole render -- so every marker in a failed string is
reported, not only the undefined one. Reporting just the bad reference
would imply the others had been substituted.

ADR 0001 now states the contract, alongside the action registry from
#241, since neither was written down.

Blocking suite 435 -> 456 passed, 13 skipped, 0 failed, 0 xfailed.
Collection 3230 -> 3251, 0 warnings. lock, ruff on src, and
`git diff --check` clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jeremymanning
jeremymanning merged commit ed51346 into main Aug 1, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fail on unresolved template references instead of writing literal {{ ... }} and exiting 0

1 participant