Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog.d/579.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
**Template filter `fromjson`**: added a `fromjson` Jinja filter that parses
JSON text into native values, the inverse of the existing `json` filter. MCP
step results (for example `output.content[0].text`) can now be parsed inside
templates to build arguments for downstream steps.
24 changes: 24 additions & 0 deletions docs/workflow-syntax.md
Original file line number Diff line number Diff line change
Expand Up @@ -1473,6 +1473,30 @@ When the tool returns structured content (a dictionary under `structured`), its
{{ read_spec.output.record_id }} # Merged structured field
```

Many widely used MCP servers return data **only** as a JSON string in
`content[0].text` and never populate `structuredContent` (`structured` stays
`null`). Use the `fromjson` filter — the inverse of `json` — to parse that
text into native data for downstream arguments and route conditions:

```jinja2
{{ (get_mr.output.content[0].text | fromjson).labels | tojson }} # Parse, then re-serialize for an arguments value
```

When the parsed value feeds an `arguments` mapping, keep it JSON: a bare
`{{ ... }}` interpolation stringifies the collection with Python `repr`
(single quotes, `None` instead of `null`, escaped newlines) before the YAML
coercion pass, corrupting values, while `tojson` emits valid JSON that the
coercion pass parses back into the same native value. In route conditions,
test the collection itself rather than interpolating it — an empty list
renders as the truthy string `"[]"`:

```jinja2
{{ (get_mr.output.content[0].text | fromjson).labels | length > 0 }} # Route condition: fire only when labels exist
```

`fromjson` raises a `TemplateError` explaining whether the value was not a
string or did not hold valid JSON.

The base keys `content`, `structured`, and `is_error` are reserved by the envelope, and `outputs` / `errors` are additionally reserved because the workflow engine recognizes parallel/for-each group outputs by exactly those two top-level keys — a structured result flattening them would make the step's output indistinguishable from a group output. If the structured dictionary contains colliding keys, the envelope wins, the colliding keys are omitted from the merge with a debug-level log message, and they stay reachable under `output.structured.<key>`.

**`is_error` semantics and routing:**
Expand Down
1 change: 1 addition & 0 deletions plugins/conductor/skills/conductor/references/authoring.md
Original file line number Diff line number Diff line change
Expand Up @@ -1116,6 +1116,7 @@ Previous: {{ previous_agent.output.result }}
{{ value | default("fallback") }} # Default value
{{ items | join(", ") }} # Join array
{{ data | json }} # JSON serialize
{{ text | fromjson }} # Parse a JSON string into native data
```

## Output Schema
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,7 @@ output:
{{ value | length }} # Length
{{ value | join(", ") }} # Join array
{{ value | json }} # JSON serialize
{{ value | fromjson }} # Parse a JSON string into native data
```

## Route Conditions
Expand Down
40 changes: 40 additions & 0 deletions src/conductor/executor/template.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ def _register_filters(self, env: Environment) -> None:
"""Register custom filters on the Jinja2 environment."""
env.filters["json"] = self._json_filter
env.filters["default"] = self._default_filter
env.filters["fromjson"] = self._fromjson_filter

@staticmethod
def _json_filter(value: Any, indent: int = 2) -> str:
Expand All @@ -117,6 +118,36 @@ def _json_filter(value: Any, indent: int = 2) -> str:
"""
return json.dumps(value, indent=indent, default=str)

@staticmethod
def _fromjson_filter(value: Any) -> Any: # noqa: ANN401
"""Parse a JSON string into native data.

Inverse of the ``json`` filter: MCP step results and other JSON text
payloads (e.g. ``output.content[0].text``) can be parsed in templates
to build arguments for downstream steps.

Args:
value: JSON string to parse.

Returns:
The parsed JSON value (dict, list, str, int, float, bool, None).

Raises:
TemplateError: If the value is not a string or holds invalid JSON.
"""
if not isinstance(value, str):
raise TemplateError(
f"fromjson filter expects a JSON string, got {type(value).__name__}",
suggestion="Apply fromjson to a text field such as output.content[0].text",
)
try:
return json.loads(value)
except json.JSONDecodeError as e:
raise TemplateError(
f"fromjson filter received invalid JSON: {e}",
suggestion="Check that the source field holds a JSON document",
) from e

@staticmethod
def _default_filter(value: Any, default: Any = "", boolean: bool = False) -> Any:
"""Return default if value is None or undefined.
Expand Down Expand Up @@ -223,6 +254,15 @@ def render(self, template: str, context: dict[str, Any]) -> str:
suggestion="Check template syntax for Jinja2 compatibility",
template_string=template,
) from e
except TemplateError as e:
# A filter raised a TemplateError carrying its own specific
# suggestion (e.g. ``fromjson`` on non-string input or invalid
# JSON) — preserve that contract instead of burying it under the
# generic wrapper below. Attach the rendered template for context
# when the filter had none.
if e.template_string is None:
e.template_string = template
raise
except Exception as e:
raise TemplateError(
f"Template rendering failed: {e}",
Expand Down
34 changes: 34 additions & 0 deletions tests/test_executor/test_mcp_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import pytest

from conductor.config.schema import MCPStepDef
from conductor.engine.context import WorkflowContext
from conductor.exceptions import ExecutionError
from conductor.executor.mcp_step import McpStepExecutor, mcp_result_bytes
from conductor.file_string import FileString
Expand Down Expand Up @@ -139,6 +140,39 @@ async def test_empty_render_binds_empty_string_not_none(
await executor.execute(agent, {}, manager) # type: ignore[arg-type]
assert manager.calls[0][2] == {"q": ""}

async def test_fromjson_parses_prior_step_text_into_native_argument(
self, executor: McpStepExecutor
) -> None:
# Requirement: the issue #579 read-modify-write pattern — a prior MCP
# step's content[0].text JSON string is parsed with ``fromjson``,
# merged, serialized with ``tojson``, and auto-coercion delivers a
# native list argument to the tool. The context is built through the
# real WorkflowContext API so the test pins the access path the
# engine actually exposes (<step>.output..., no steps. prefix).
manager = FakeMCPManager()
agent = make_agent(
arguments={
"labels": (
"{{ (((get_mr.output.content[0].text | fromjson).labels"
" | default([])) + ['Conductor::Need human']) | list | tojson }}"
)
}
)
workflow_context = WorkflowContext()
workflow_context.store(
"get_mr",
{
"content": [{"type": "text", "text": '{"labels": ["renovate"]}'}],
"structured": None,
"is_error": False,
},
)
context = workflow_context.build_for_agent("update_mr", [])
await executor.execute(agent, context, manager) # type: ignore[arg-type]
labels = manager.calls[0][2]["labels"]
assert labels == ["renovate", "Conductor::Need human"]
assert isinstance(labels, list)

async def test_no_arguments_sends_empty_dict(self, executor: McpStepExecutor) -> None:
# Requirement: steps without arguments call the tool with an empty dict.
manager = FakeMCPManager()
Expand Down
99 changes: 99 additions & 0 deletions tests/test_executor/test_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@
- Condition evaluation
"""

import json

import pytest

from conductor.exceptions import TemplateError
from conductor.executor.set_step import _yaml_load
from conductor.executor.template import TemplateRenderer


Expand Down Expand Up @@ -90,6 +93,102 @@ def test_json_filter_handles_non_serializable(self) -> None:
assert "2024-01-01" in result


class TestTemplateRendererFromjsonFilter:
"""Tests for the fromjson filter (JSON text to native values)."""

def test_fromjson_parses_object(self) -> None:
"""Test parsing a JSON object string for key access."""
renderer = TemplateRenderer()
result = renderer.render(
"{{ (text | fromjson).labels }}",
{"text": '{"labels": ["renovate", "dependencies"]}'},
)
assert result == "['renovate', 'dependencies']"

def test_fromjson_parses_array_with_index(self) -> None:
"""Test parsing a JSON array string for element access."""
renderer = TemplateRenderer()
result = renderer.render(
"{{ (items | fromjson)[1] }}",
{"items": '["first", "second"]'},
)
assert result == "second"

def test_fromjson_scalars(self) -> None:
"""Test parsing JSON scalar strings."""
renderer = TemplateRenderer()
assert renderer.render("{{ n | fromjson }}", {"n": "42"}) == "42"
assert renderer.render("{{ b | fromjson }}", {"b": "true"}) == "True"
assert renderer.render("{{ s | fromjson }}", {"s": '"hello"'}) == "hello"

def test_fromjson_composes_with_default_and_concat(self) -> None:
"""Test the MCP-step argument pattern: parse text, default, append."""
renderer = TemplateRenderer()
result = renderer.render(
"{{ ((text | fromjson).labels | default([])) + ['Conductor::Need human'] }}",
{"text": '{"labels": ["renovate"]}'},
)
assert result == "['renovate', 'Conductor::Need human']"

def test_fromjson_collection_round_trips_through_tojson_not_bare_repr(self) -> None:
"""Test that a parsed collection must be re-serialized, not interpolated bare.

Requirement: MCP-step argument coercion YAML-parses the rendered
string, so a bare ``{{ ... }}`` interpolation of a Python collection
corrupts it (``None`` becomes the string "None", embedded newlines
stay repr-escaped), while ``tojson`` emits JSON that parses back into
the same native value.
"""
renderer = TemplateRenderer()
text = '{"labels": [null, "a\\nb"]}'
bare = renderer.render("{{ (text | fromjson).labels }}", {"text": text})
assert _yaml_load(bare) == ["None", "a\\nb"]
via_tojson = renderer.render("{{ (text | fromjson).labels | tojson }}", {"text": text})
assert _yaml_load(via_tojson) == [None, "a\nb"]

def test_fromjson_empty_collection_is_truthy_when_interpolated_bare(self) -> None:
"""Test that route conditions must test a collection, not interpolate it.

Requirement: an empty list rendered bare produces the string "[]",
which a route condition reads as truthy — conditions must use a
predicate such as ``length > 0`` on the parsed collection instead.
"""
renderer = TemplateRenderer()
text = '{"labels": []}'
assert renderer.evaluate_condition("{{ (text | fromjson).labels }}", {"text": text})
assert not renderer.evaluate_condition(
"{{ (text | fromjson).labels | length > 0 }}", {"text": text}
)

def test_fromjson_invalid_json_raises(self) -> None:
"""Test that invalid JSON raises a TemplateError.

Requirement: the filter's own error contract (specific suggestion,
original template, and the JSONDecodeError cause) must survive the
renderer's exception wrapping unchanged.
"""
renderer = TemplateRenderer()
with pytest.raises(TemplateError, match="invalid JSON") as exc_info:
renderer.render("{{ text | fromjson }}", {"text": "not json"})
error = exc_info.value
assert error.suggestion == "Check that the source field holds a JSON document"
assert error.template_string == "{{ text | fromjson }}"
assert isinstance(error.__cause__, json.JSONDecodeError)

def test_fromjson_non_string_raises(self) -> None:
"""Test that applying fromjson to a non-string raises a TemplateError.

Requirement: the filter's specific suggestion must reach the caller
rather than the renderer's generic "Check template and context" one.
"""
renderer = TemplateRenderer()
with pytest.raises(TemplateError, match="expects a JSON string") as exc_info:
renderer.render("{{ value | fromjson }}", {"value": 42})
error = exc_info.value
assert error.suggestion == ("Apply fromjson to a text field such as output.content[0].text")
assert error.template_string == "{{ value | fromjson }}"


class TestTemplateRendererDefaultFilter:
"""Tests for the default filter."""

Expand Down
Loading