diff --git a/acc/cli/plan_cmd.py b/acc/cli/plan_cmd.py index 1a408ca7..7d050ed4 100644 --- a/acc/cli/plan_cmd.py +++ b/acc/cli/plan_cmd.py @@ -94,13 +94,12 @@ async def _cmd_submit(args: argparse.Namespace) -> int: raw = _read_plan(args.plan_file) if raw is None: return 1 - try: - payload = json.loads(raw) - except json.JSONDecodeError as exc: - print(f"plan: invalid JSON in {args.plan_file!r}: {exc}", file=sys.stderr) + payload = _parse_plan_text(raw, str(args.plan_file)) + if payload is None: return 1 if not isinstance(payload, dict): - print("plan: payload must be a JSON object", file=sys.stderr) + print("plan: payload must be a mapping (JSON object / YAML map)", + file=sys.stderr) return 1 cid = args.collective or payload.get("collective_id") or default_collective() @@ -242,3 +241,54 @@ def _read_plan(plan_file: str) -> str | None: except OSError as exc: print(f"plan: read failed: {exc}", file=sys.stderr) return None + + +def _parse_plan_text(raw: str, path_hint: str) -> Any: + """Parse a PLAN payload from JSON or YAML text. + + Routing: + + * Files ending ``.yaml`` / ``.yml`` parse via PyYAML directly. + A YAML parse error is fatal — a YAML-extension file should + always be valid YAML. + * Anything else attempts JSON first. On JSONDecodeError, the + content is retried as YAML — JSON is itself a YAML subset, so + this lets ``acc-cli plan submit -`` accept either dialect from + stdin without a flag. + + Returns ``None`` on parse failure; the handler converts that to + exit code 1 and prints a diagnostic. + + YAML support lets operators author plans next to the role.md + sources (PR #28) without an extra json-conversion step. + """ + is_yaml_path = path_hint.lower().endswith((".yaml", ".yml")) + if is_yaml_path: + try: + import yaml # noqa: PLC0415 — already a project dep + except ImportError: # pragma: no cover — defensive + print("plan: PyYAML not installed; cannot parse .yaml plan file", + file=sys.stderr) + return None + try: + return yaml.safe_load(raw) + except yaml.YAMLError as exc: + print(f"plan: invalid YAML in {path_hint!r}: {exc}", + file=sys.stderr) + return None + + # JSON-first path (back-compat: pre-existing JSON plan files keep + # working; stdin defaults here too). + try: + return json.loads(raw) + except json.JSONDecodeError as exc: + # Permissive fallback: a stdin-piped YAML or a comment-bearing + # JSON-ish file may still parse cleanly as YAML. + try: + import yaml # noqa: PLC0415 + return yaml.safe_load(raw) + except (ImportError, Exception): + pass + print(f"plan: invalid JSON in {path_hint!r}: {exc}", + file=sys.stderr) + return None diff --git a/tests/test_cli_plan_yaml.py b/tests/test_cli_plan_yaml.py new file mode 100644 index 00000000..79e39b36 --- /dev/null +++ b/tests/test_cli_plan_yaml.py @@ -0,0 +1,117 @@ +"""``acc-cli plan submit`` accepts both JSON and YAML plan files (D2). + +The handler previously called ``json.loads`` directly which rejected +the YAML scenario plans landed in PR #29's docs work. This test +module pins both formats + the malformed-input contract. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from acc.cli.plan_cmd import _parse_plan_text + + +_VALID_PLAN_DICT = { + "signal_type": "PLAN", + "plan_id": "test-plan-1", + "collective_id": "sol-01", + "steps": [ + {"step_id": "s1", "role": "coding_agent", + "depends_on": [], "task_description": "noop"}, + ], +} + + +# --------------------------------------------------------------------------- +# JSON +# --------------------------------------------------------------------------- + + +def test_json_plan_parses(tmp_path: Path): + p = tmp_path / "plan.json" + p.write_text(json.dumps(_VALID_PLAN_DICT), encoding="utf-8") + out = _parse_plan_text(p.read_text(encoding="utf-8"), str(p)) + assert isinstance(out, dict) + assert out["plan_id"] == "test-plan-1" + assert out["steps"][0]["role"] == "coding_agent" + + +def test_json_plan_with_arbitrary_extension_falls_through(tmp_path: Path): + """A path without .yaml/.yml hits the JSON-first branch.""" + p = tmp_path / "plan.txt" + p.write_text(json.dumps(_VALID_PLAN_DICT), encoding="utf-8") + out = _parse_plan_text(p.read_text(encoding="utf-8"), str(p)) + assert isinstance(out, dict) + assert out["plan_id"] == "test-plan-1" + + +# --------------------------------------------------------------------------- +# YAML +# --------------------------------------------------------------------------- + + +_VALID_PLAN_YAML = """\ +signal_type: "PLAN" +plan_id: "test-plan-yaml" +collective_id: "sol-01" +steps: + - step_id: "s1" + role: "coding_agent" + depends_on: [] + task_description: | + Generate a simple module. +""" + + +def test_yaml_plan_parses_with_yaml_extension(tmp_path: Path): + p = tmp_path / "plan.yaml" + p.write_text(_VALID_PLAN_YAML, encoding="utf-8") + out = _parse_plan_text(p.read_text(encoding="utf-8"), str(p)) + assert isinstance(out, dict) + assert out["plan_id"] == "test-plan-yaml" + assert out["steps"][0]["task_description"].startswith("Generate") + + +def test_yaml_plan_parses_with_yml_extension(tmp_path: Path): + p = tmp_path / "plan.yml" + p.write_text(_VALID_PLAN_YAML, encoding="utf-8") + out = _parse_plan_text(p.read_text(encoding="utf-8"), str(p)) + assert isinstance(out, dict) + + +def test_yaml_extension_fails_loudly_on_invalid_yaml(tmp_path: Path, capsys): + """A .yaml file with broken YAML must NOT silently fall through to + JSON — a yaml-extension file should always be YAML. Operators + expect a yaml-shaped diagnostic in this case.""" + p = tmp_path / "broken.yaml" + p.write_text("steps: [\n not valid: yaml: : :", encoding="utf-8") + out = _parse_plan_text(p.read_text(encoding="utf-8"), str(p)) + assert out is None + captured = capsys.readouterr() + assert "invalid YAML" in captured.err + + +# --------------------------------------------------------------------------- +# Stdin / generic fallback +# --------------------------------------------------------------------------- + + +def test_yaml_text_via_unknown_extension_falls_back(tmp_path: Path): + """A YAML body served from stdin (path_hint == '-') or a + non-yaml extension must still parse via the JSON-fallback branch. + """ + out = _parse_plan_text(_VALID_PLAN_YAML, "-") + assert isinstance(out, dict) + assert out["plan_id"] == "test-plan-yaml" + + +def test_malformed_input_returns_none(capsys): + out = _parse_plan_text("{[ ! not parseable as either }", + "stdin.json") + assert out is None + err = capsys.readouterr().err + assert "invalid JSON" in err