diff --git a/README.md b/README.md index c6df63e..aa59723 100644 --- a/README.md +++ b/README.md @@ -687,10 +687,23 @@ and Pydantic input/output schemas. Collection dependencies include their kind (`positional` or `keyed`) and member references. `python_type` identifies the loaded class; it may differ from the plugin entry-point identifier used in YAML. -Pass `--input input.yaml` to check that required configuration fields and map -sources are supplied. Then `provided_config_fields` lists **field names only**; -without `--input`, it is `null`. This check does not run tasks or hooks and does -not validate every configured value's runtime type. Loading YAML still imports +Pass `--input input.yaml` to check the input against the workflow. The input +may be incomplete (for example a template whose runtime-picker fields a UI +fills in): `provided_config_fields` lists the field names set for each task, +`missing_config_fields` lists declared `config_fields` the input does not set, +and `config_values` maps exactly the provided names to their values; without +`--input`, all three are `null`. Other problems, such as unknown task keys or a +missing or invalid map source, still fail. `validate` and `run` reject missing +configuration fields. Values are what `input.yaml` contains for +the task, **before** Pydantic validation, so opaque markers (for example +`{"__resinsight_ref__": "EclipseCase", "case_id": 0}`) come through unchanged. +Dates and datetimes become ISO 8601 strings, paths become strings, sets become +sorted lists and non-string mapping keys become JSON key strings; binary values, +NaN/infinity and keys that collide after conversion are rejected as invalid. +Fields without a value fall back to `input_schema.properties..default`. +**The output contains the input values, including any secrets.** This check +does not run tasks or hooks and does not validate every configured value's +runtime type. Loading YAML still imports Python modules (and nested workflows); **do not inspect untrusted YAML or plugins** under a privileged account. In JSON mode inspection failures have status `invalid`, an `error` object, and exit code 2. Without `--json`, inspection diff --git a/docs/agent-quickstart.md b/docs/agent-quickstart.md index 26d25ad..6567f92 100644 --- a/docs/agent-quickstart.md +++ b/docs/agent-quickstart.md @@ -86,7 +86,7 @@ Inspection needs only the workflow YAML, so use it to find required fields and r .venv/bin/python -m taskmaestro workflow describe examples/agent_quickstart/workflow.yaml --json ``` -This reports the result task `double`, a root `add_one` with `config_fields: ["value"]`, the dependency on `add_one`, and schemas for each task. Add `--input examples/agent_quickstart/input.yaml` to check that required configuration fields are present; the output includes `provided_config_fields` **names**, not their values. Inspection does not execute tasks or instantiate hooks, but it **does import Python modules** named in YAML (including nested workflows). Only inspect trusted workflow files and plugins. Runtime input values are checked when the tasks run, not completely by inspection. +This reports the result task `double`, a root `add_one` with `config_fields: ["value"]`, the dependency on `add_one`, and schemas for each task. Add `--input examples/agent_quickstart/input.yaml` to check an input file, which may still be incomplete; each task then reports `provided_config_fields` (names), `missing_config_fields` (declared `config_fields` the input does not set yet; `validate` and `run` reject these) and `config_values` (the raw values from `input.yaml`, before Pydantic validation, with dates as ISO 8601 strings). The output therefore **contains the input values, including any secrets**. Inspection does not execute tasks or instantiate hooks, but it **does import Python modules** named in YAML (including nested workflows). Only inspect trusted workflow files and plugins. Runtime input values are checked when the tasks run, not completely by inspection. ## Parse results and errors as JSON diff --git a/taskmaestro/cli.py b/taskmaestro/cli.py index a0fa09b..5093eec 100644 --- a/taskmaestro/cli.py +++ b/taskmaestro/cli.py @@ -5,11 +5,13 @@ import argparse import json import logging +import math import sys from collections.abc import Iterator, Sequence from contextlib import contextmanager, redirect_stdout from dataclasses import asdict -from pathlib import Path +from datetime import date +from pathlib import Path, PurePath from typing import Any from pydantic import ValidationError @@ -20,7 +22,7 @@ from taskmaestro.dependencies import CollectionRef, OutputRef from taskmaestro.discovery import get_registered_task, registered_task_names from taskmaestro.exceptions import ConfigLoadError, PluginLoadError, WorkflowDefinitionError -from taskmaestro.job import EmptyConfig, Job, JobStatus +from taskmaestro.job import EmptyConfig, Job, JobConfiguration, JobStatus from taskmaestro.task import get_input_type, get_output_type from taskmaestro.workflow import Workflow from taskmaestro.yaml_config import LoadedWorkflow, _load_workflow_only, load_workflow_from_yaml @@ -209,8 +211,87 @@ def _dependency_spec(deps: Any) -> Any: return result +def _jsonable_key(key: object, path: str) -> str: + """Convert a YAML mapping key to the JSON object key it would get.""" + value = _jsonable(key, path) + if isinstance(value, str): + return value + if isinstance(value, bool | int | float) or value is None: + return json.dumps(value) + raise TypeError(f"Configuration value '{path}' has an unsupported mapping key type") + + +def _jsonable(value: object, path: str) -> Any: + """Convert a raw configuration value (as loaded from YAML) to plain JSON data. + + Dates and datetimes become ISO 8601 strings and paths become strings; + everything else must already be JSON-compatible. ``path`` names the value + in error messages, which deliberately never include the value itself. + """ + if value is None or isinstance(value, bool | int | str): + return value + if isinstance(value, float): + if not math.isfinite(value): + raise ValueError(f"Configuration value '{path}' is not a finite number") + return value + if isinstance(value, date): # Includes datetime. + return value.isoformat() + if isinstance(value, PurePath): + return str(value) + if isinstance(value, dict): + result: dict[str, Any] = {} + for key, item in value.items(): + json_key = _jsonable_key(key, path) + if json_key in result: + raise ValueError( + f"Configuration value '{path}' has keys that collide as JSON key '{json_key}'" + ) + result[json_key] = _jsonable(item, f"{path}.{json_key}") + return result + if isinstance(value, list | tuple): + return [_jsonable(item, f"{path}[{index}]") for index, item in enumerate(value)] + if isinstance(value, set | frozenset): + items = [_jsonable(item, f"{path}[]") for item in value] + try: + return sorted(items) + except TypeError: + # Mixed types: order by canonical JSON text so output is deterministic. + return sorted(items, key=lambda item: json.dumps(item, sort_keys=True)) + raise TypeError(f"Configuration value '{path}' has unsupported type {type(value).__name__}") + + +def _missing_config_fields(workflow: Workflow, config: JobConfiguration) -> dict[str, list[str]]: + """Return the declared config fields that the input does not supply, per task.""" + return { + name: sorted(workflow.get_config_fields(name) - config.config_fields_for_task(name)) + for name, _task in workflow.topological_order() + } + + +def _fill_missing( + workflow: Workflow, config: JobConfiguration, missing: dict[str, list[str]] +) -> JobConfiguration: + """Return a copy of ``config`` with placeholder values for missing config fields. + + ``Job`` only checks that these fields are present, so placeholders let it run + its remaining validation. Map sources get no placeholder, so a missing one is + still rejected with its own error. + """ + filled: dict[str, dict[str, Any]] = {} + for name, fields in missing.items(): + task_map = workflow.get_task_map(name) + map_source = task_map.over if task_map is not None else None + placeholders = {field: None for field in fields if field != map_source} + filled[name] = {**placeholders, **config.get_config_for_task(name)} + return JobConfiguration(filled) + + def _workflow_description( - workflow: Workflow, *, configured: dict[str, list[str]] | None + workflow: Workflow, + *, + configured: dict[str, list[str]] | None, + missing: dict[str, list[str]] | None = None, + values: dict[str, dict[str, Any]] | None = None, ) -> dict[str, Any]: tasks: list[dict[str, Any]] = [] for name, task in workflow.topological_order(): @@ -224,6 +305,8 @@ def _workflow_description( "depends_on": _dependency_spec(workflow.get_dependencies(name)), "config_fields": sorted(workflow.get_config_fields(name)), "provided_config_fields": configured[name] if configured is not None else None, + "missing_config_fields": missing[name] if missing is not None else None, + "config_values": values[name] if values is not None else None, "required_input_fields": sorted( field for field, info in input_type.model_fields.items() if info.is_required() ), @@ -246,15 +329,36 @@ def _workflow_describe(args: argparse.Namespace) -> int: Path(args.workflow), Path(args.input) if args.input else None ) configured = None + missing = None + values = None if args.input is not None: assert config is not None - # Check supplied config without constructing hooks or executing any tasks. - Job(workflow, EmptyConfig(), job_configuration=config) + missing = _missing_config_fields(workflow, config) + # Check the rest of the supplied config (map sources, root inputs) + # without constructing hooks or executing any tasks. Missing config + # fields are reported rather than rejected: the input may be a + # template whose remaining fields the caller fills in. + Job( + workflow, + EmptyConfig(), + job_configuration=_fill_missing(workflow, config, missing), + ) configured = { name: sorted(config.config_fields_for_task(name)) for name, _task in workflow.topological_order() } - description = _workflow_description(workflow, configured=configured) + # Raw input values, before Pydantic validation, so opaque or + # runtime-only markers pass through unchanged. + values = { + name: { + field: _jsonable(value, f"{name}.{field}") + for field, value in sorted(config.get_config_for_task(name).items()) + } + for name, _task in workflow.topological_order() + } + description = _workflow_description( + workflow, configured=configured, missing=missing, values=values + ) except WorkflowDefinitionError as exc: raise ConfigLoadError(f"Job validation failed: {exc}") from exc except ( @@ -336,7 +440,8 @@ def build_parser() -> argparse.ArgumentParser: ) workflow_describe_parser.add_argument("workflow", help="Path to the workflow YAML file") workflow_describe_parser.add_argument( - "--input", help="Optional input YAML to check required configuration fields" + "--input", + help="Optional input YAML; reports provided, missing and configured values per task", ) workflow_describe_parser.add_argument( "--json", action="store_true", help="Print single-line JSON" diff --git a/tests/test_cli.py b/tests/test_cli.py index 90bcb1d..8afd006 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -6,13 +6,14 @@ import sys from importlib.metadata import EntryPoint from pathlib import Path +from typing import Any import pytest from pydantic import BaseModel from pydantic_core import core_schema from taskmaestro import ExecutionContext, ObjectModel, Task -from taskmaestro.cli import _dependency_spec, main +from taskmaestro.cli import _dependency_spec, _jsonable, main from taskmaestro.dependencies import CollectionRef, OutputRef @@ -49,6 +50,38 @@ def run(self, input: UnsupportedInput, ctx: ExecutionContext) -> UnsupportedInpu return input +class ConfigValuesInput(BaseModel): + """Accepts arbitrary values so tests can exercise raw config value reporting.""" + + value: Any = None + end_date: Any = None + started: Any = None + grid_case: Any = None + steps: Any = None + by_number: Any = None + tags: Any = None + extra: Any = None + + +class ConfigValuesTask(Task[ConfigValuesInput, ConfigValuesInput]): + name = "config_values" + + def run(self, input: ConfigValuesInput, ctx: ExecutionContext) -> ConfigValuesInput: + return input + + +def _config_values_files(tmp_path: Path, values: str) -> tuple[Path, Path]: + workflow = tmp_path / "workflow.yaml" + workflow.write_text( + "workflow:\n name: config_values\n tasks:\n" + " - task: tests.test_cli.ConfigValuesTask\n", + encoding="utf-8", + ) + input_path = tmp_path / "input.yaml" + input_path.write_text(f"config_values:\n{values}", encoding="utf-8") + return workflow, input_path + + def _files(tmp_path: Path, task: str = "Increment") -> tuple[Path, Path]: (tmp_path / "pipeline.py").write_text( """\ @@ -399,6 +432,8 @@ def test_workflow_describe_without_input( assert task["required_input_fields"] == ["value"] assert task["config_fields"] == [] assert task["provided_config_fields"] is None + assert task["missing_config_fields"] is None + assert task["config_values"] is None assert task["input_schema"]["properties"]["value"]["type"] == "integer" assert task["output_schema"]["properties"]["value"]["type"] == "integer" assert main(["workflow", "describe", str(workflow)]) == 0 @@ -435,7 +470,99 @@ def test_workflow_describe_with_input_does_not_execute( assert main(["workflow", "describe", str(workflow), "--input", str(input_path), "--json"]) == 0 description = json.loads(capsys.readouterr().out) assert description["tasks"][0]["provided_config_fields"] == ["value"] - assert "private-token" not in json.dumps(description) + assert description["tasks"][0]["missing_config_fields"] == [] + # Values are reported raw, before validation (the task expects an int). + assert description["tasks"][0]["config_values"] == {"value": "private-token"} + + +def test_workflow_describe_reports_raw_config_values( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + workflow, input_path = _config_values_files( + tmp_path, + """\ + value: 4 + end_date: 2030-01-01 + started: 2030-01-01T12:30:00Z + grid_case: {__resinsight_ref__: EclipseCase, case_id: 0} + steps: [1, 2.5, null, true] + by_number: {1: one, 2030-01-02: date, false: f, null: n, 1.5: x} + tags: !!set {b: null, a: null} +""", + ) + + assert main(["workflow", "describe", str(workflow), "--input", str(input_path), "--json"]) == 0 + task = json.loads(capsys.readouterr().out)["tasks"][0] + assert task["config_values"] == { + "by_number": {"1": "one", "2030-01-02": "date", "false": "f", "null": "n", "1.5": "x"}, + "end_date": "2030-01-01", + "grid_case": {"__resinsight_ref__": "EclipseCase", "case_id": 0}, + "started": "2030-01-01T12:30:00+00:00", + "steps": [1, 2.5, None, True], + "tags": ["a", "b"], + "value": 4, + } + assert list(task["config_values"]) == task["provided_config_fields"] + + +@pytest.mark.parametrize( + ("yaml_value", "message"), + [ + (".nan", "not a finite number"), + ("!!binary aGVsbG8=", "unsupported type bytes"), + ("{1: a, '1': b}", "collide"), + ], +) +def test_workflow_describe_rejects_unsupported_config_values( + tmp_path: Path, capsys: pytest.CaptureFixture[str], yaml_value: str, message: str +) -> None: + workflow, input_path = _config_values_files(tmp_path, f" value: 4\n extra: {yaml_value}\n") + + assert main(["workflow", "describe", str(workflow), "--input", str(input_path), "--json"]) == 2 + result = json.loads(capsys.readouterr().out) + assert result["status"] == "invalid" + assert result["error"]["code"] == "configuration_error" + + assert main(["workflow", "describe", str(workflow), "--input", str(input_path)]) == 2 + err = capsys.readouterr().err + assert "'config_values.extra'" in err + assert message in err + + +def test_jsonable_handles_values_yaml_cannot_express() -> None: + """Programmatic configs may hold paths, tuple keys or mixed-type sets.""" + assert _jsonable({"out": Path("/tmp/out"), "pair": (1, 2)}, "task") == { + "out": "/tmp/out", + "pair": [1, 2], + } + assert _jsonable({1, "a", None}, "task.tags") == ["a", 1, None] # By JSON text. + with pytest.raises(TypeError, match="'task' has an unsupported mapping key type"): + _jsonable({(1, 2): "x"}, "task") + + +def test_workflow_describe_config_values_for_partial_and_mapped_config( + capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch +) -> None: + example = Path(__file__).resolve().parents[1] / "examples/release_pipeline" + + with monkeypatch.context() as patch: + patch.delitem(sys.modules, "pipeline", raising=False) + args = ["workflow", "describe", str(example / "workflow.yaml")] + assert main([*args, "--input", str(example / "input.yaml"), "--json"]) == 0 + patch.delitem(sys.modules, "pipeline", raising=False) + tasks = {task["name"]: task for task in json.loads(capsys.readouterr().out)["tasks"]} + for task in tasks.values(): + assert list(task["config_values"]) == task["provided_config_fields"] + assert task["missing_config_fields"] == [] + # The mapped task's map source comes from input.yaml, so it is reported. + targets = tasks["build_targets"]["config_values"]["targets"] + assert targets["linux-x64"] == { + "operating_system": "linux", + "architecture": "x86_64", + "extension": "tar.gz", + } + assert tasks["load_package"]["config_values"]["version"] == "1.0.0" + assert tasks["validate_release"]["config_values"] == {} def test_workflow_describe_missing_input_reports_json_error( @@ -453,18 +580,49 @@ def test_workflow_describe_missing_input_reports_json_error( def test_workflow_describe_reports_missing_config_fields( tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: - workflow, input_path = _files(tmp_path) + """An incomplete input (e.g. a template with runtime pickers) is described, not rejected.""" + workflow, input_path = _config_values_files(tmp_path, " end_date: 2030-01-01\n") workflow.write_text( - "workflow:\n name: cli_test\n tasks:\n" - " - task: pipeline.Increment\n config_fields: [value]\n", + workflow.read_text(encoding="utf-8") + + " config_fields: [end_date, grid_case, value]\n", encoding="utf-8", ) - input_path.write_text("{}\n", encoding="utf-8") - assert main(["workflow", "describe", str(workflow), "--input", str(input_path), "--json"]) == 2 + assert main(["workflow", "describe", str(workflow), "--input", str(input_path), "--json"]) == 0 + task = json.loads(capsys.readouterr().out)["tasks"][0] + assert task["config_fields"] == ["end_date", "grid_case", "value"] + assert task["provided_config_fields"] == ["end_date"] + assert task["missing_config_fields"] == ["grid_case", "value"] + assert task["config_values"] == {"end_date": "2030-01-01"} + + # Validation and execution stay strict about missing configuration. + assert main(["validate", str(workflow), "--input", str(input_path), "--json"]) == 2 result = json.loads(capsys.readouterr().out) - assert result["error"]["task"] == "increment" - assert result["error"]["issues"] == [{"field": "value", "code": "missing"}] + assert result["error"]["task"] == "config_values" + assert result["error"]["issues"] == [ + {"field": "grid_case", "code": "missing"}, + {"field": "value", "code": "missing"}, + ] + + +def test_workflow_describe_rejects_missing_map_source( + tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch +) -> None: + example = Path(__file__).resolve().parents[1] / "examples/release_pipeline" + input_path = tmp_path / "input.yaml" + input_path.write_text( + (example / "input.yaml").read_text(encoding="utf-8").split("build_targets:")[0], + encoding="utf-8", + ) + + with monkeypatch.context() as patch: + patch.delitem(sys.modules, "pipeline", raising=False) + args = ["workflow", "describe", str(example / "workflow.yaml"), "--input"] + assert main([*args, str(input_path)]) == 2 + patch.delitem(sys.modules, "pipeline", raising=False) + assert "Mapped task 'build_targets' requires configuration field 'targets'" in ( + capsys.readouterr().err + ) def test_workflow_dependency_routing_and_positional_collection() -> None: