Answer RECORD_INVALID for an invalid decision record

CHANGELOG.md

@@ -392,6 +392,12 @@ Notable changes to Cloudfall. The format follows
 
 ### Fixed
 
+- A decision record that is not JSON or does not match its schema answers
+  `RECORD_INVALID` (exit 86, code `decision_record_invalid`) naming the file
+  and the failing field, from `decisions show`, `decisions list`,
+  `decisions approve` and `why`; it used to crash them with exit 1 and no
+  envelope. `decisions list` and `why` still stop at the first bad record
+  rather than skip it (#41)
 - `migrate --env-file NAME=PATH` and `--data NAME=PATH` resolve a relative
   path against the project again, as every other path does; since the
   move to treaty they resolved against the current directory

sdk/src/cloudfall/decision.py

@@ -27,6 +27,8 @@
 from pathlib import Path
 from typing import TYPE_CHECKING, cast
 
+from jsonschema.exceptions import ValidationError
+
 from cloudfall.catalog import RiskLevel, TargetScope
 from cloudfall.domain import ResourceId
 
@@ -69,6 +71,7 @@
 _ERROR_NOT_PROPOSED = "decision_not_proposed"
 _ERROR_APPROVER_UNKNOWN = "decision_approver_unknown"
 _ERROR_CHANNEL_UNKNOWN = "decision_approval_channel_unknown"
+_ERROR_RECORD_INVALID = "decision_record_invalid"
 
 
 class DecisionError(RuntimeError):
@@ -353,10 +356,22 @@ def load(self, decision_id: ResourceId) -> Decision:
         if not path.is_file():
             message = f"decision does not exist: {path}"
             raise DecisionError(ERROR_DECISION_MISSING, message)
-        document = cast(
-            "dict[str, object]", json.loads(path.read_text(encoding="utf-8"))
-        )
-        self.catalog.validate_named(DECISION_SCHEMA, document)
+        try:
+            document = cast(
+                "dict[str, object]", json.loads(path.read_text(encoding="utf-8"))
+            )
+        except (UnicodeDecodeError, json.JSONDecodeError) as error:
+            message = f"decision record is not valid JSON: {path}: {error}"
+            raise DecisionError(_ERROR_RECORD_INVALID, message) from error
+        try:
+            self.catalog.validate_named(DECISION_SCHEMA, document)
+        except ValidationError as error:
+            field = ".".join(str(part) for part in error.absolute_path) or "(root)"
+            message = (
+                f"decision record does not match its schema: {path}: "
+                f"{field}: {error.message}"
+            )
+            raise DecisionError(_ERROR_RECORD_INVALID, message) from error
         return _decision_from_document(document)
 
     def list(self) -> tuple[Decision, ...]:

sdk/tests/test_decision.py

@@ -36,7 +36,7 @@
 from treaty import CommandPath
 
 if TYPE_CHECKING:
-    from collections.abc import Mapping, Sequence
+    from collections.abc import Callable, Mapping, Sequence
 
 SCHEMAS = default_schema_directory()
 MOMENT = datetime(2026, 9, 21, 14, 30, 12, tzinfo=UTC)
@@ -943,3 +943,95 @@ def test_every_outcome_says_why_in_the_record(tmp_path: Path) -> None:
     assert executed.verdict == (
         "the run succeeded; the operation declares no verify step"
     )
+
+
+def _not_json(path: Path) -> str:
+    path.write_text("{not json", encoding="utf-8")
+    return "not valid JSON"
+
+
+def _bad_status(path: Path) -> str:
+    document = json.loads(path.read_text(encoding="utf-8"))
+    document["spec"]["status"] = "bogus"
+    path.write_text(json.dumps(document), encoding="utf-8")
+    return "spec.status"
+
+
+def _bad_via(path: Path) -> str:
+    document = json.loads(path.read_text(encoding="utf-8"))
+    document["spec"]["approval"]["via"] = "pigeon"
+    path.write_text(json.dumps(document), encoding="utf-8")
+    return "spec.approval.via"
+
+
+RECORD_COMMANDS: dict[str, tuple[str, ...]] = {
+    "show": ("decisions", "show", "{id}"),
+    "list": ("decisions", "list"),
+    "approve": ("decisions", "approve", "{id}", "--approver", "roman"),
+    "why": ("why",),
+}
+
+
+def _run_on_record(
+    command: str, repository: Path, decision_id: str
+) -> tuple[int, dict[str, object]]:
+    """Run one record-reading command as a person at a terminal."""
+    argv = [part.replace("{id}", decision_id) for part in RECORD_COMMANDS[command]]
+    stdout = _Terminal()
+    exit_code = app.run(
+        [*argv, "--repository", str(repository), "--format", "json"],
+        stdin=_Terminal(f"{decision_id}\n"),
+        stdout=stdout,
+        stderr=io.StringIO(),
+    )
+    return exit_code, json.loads(stdout.getvalue())
+
+
+@pytest.mark.parametrize("command", sorted(RECORD_COMMANDS))
+@pytest.mark.parametrize(
+    "corrupt", [_not_json, _bad_status, _bad_via], ids=["json", "status", "via"]
+)
+def test_an_invalid_record_answers_record_invalid_naming_the_file(
+    tmp_path: Path, command: str, corrupt: Callable[[Path], str]
+) -> None:
+    """A broken record is a typed RECORD_INVALID, never a crash (#41)."""
+    repository = _repository(tmp_path)
+    store = _store(repository)
+    proposed = propose(_proposal(repository), store, lambda: MOMENT, _Runner())
+    approve(
+        ApprovalRequest(
+            decision=proposed, approver="roman", repository=repository, via=TERMINAL
+        ),
+        store,
+        lambda: MOMENT,
+        _Runner(),
+    )
+    path = repository / DECISION_DIRECTORY / f"{proposed.decision_id.value}.json"
+    named = corrupt(path)
+
+    exit_code, payload = _run_on_record(command, repository, proposed.decision_id.value)
+
+    error = payload["error"]
+    assert isinstance(error, dict)
+    assert exit_code == 86, payload
+    assert error["code"] == "RECORD_INVALID"
+    assert error["context"]["code"] == "decision_record_invalid"
+    assert str(path) in error["message"]
+    assert named in error["message"]
+
+
+def test_a_store_refuses_an_invalid_record_with_a_decision_error(
+    tmp_path: Path,
+) -> None:
+    repository = _repository(tmp_path)
+    store = _store(repository)
+    proposed = propose(_proposal(repository), store, lambda: MOMENT, _Runner())
+    path = repository / DECISION_DIRECTORY / f"{proposed.decision_id.value}.json"
+    _bad_status(path)
+
+    with pytest.raises(DecisionError) as error:
+        store.list()
+
+    assert error.value.code == "decision_record_invalid"
+    assert str(path) in error.value.detail
+    assert "spec.status" in error.value.detail