Exit PRECONDITION when the investigation record cannot be written

CHANGELOG.md

@@ -451,6 +451,14 @@ Notable changes to Cloudfall. The format follows
   stopped by a signal or a timeout keeps its own exit code, and one stopped
   while it waits on the model still leaves no record;
   `--no-stream` is unchanged (#44)
+- `agent investigate` exits 4 `PRECONDITION` (code
+  `agent_investigations_unwritable`), naming the investigations folder,
+  when it cannot write the investigation record, such as when a file
+  stands where the folder should be. A run that ended crashed with a
+  traceback instead; one whose reader closed stdout mid-run printed the
+  traceback and exited 0. Such a stopped run still exits non-zero, now
+  `PRECONDITION` with the reason as a JSON line on stderr, though its
+  record is lost (#56)
 - The operator proposal schema refuses `approval.via` on an autonomous
   (policy) approval. Such a receipt used to pass validation and lose the
   field on load; it now fails schema validation like any other invalid

sdk/src/cloudfall/app.py

@@ -2221,6 +2221,14 @@ def decisions_approve(args: ApproveDecisionArgs, ctx: Ctx) -> Decided:
 ``INVESTIGATION_STOPPED`` instead. A stream closed by a signal or a timeout
 keeps the exit code treaty gives it.
 """
+stream_unrecorded: list[InvestigationError] = []
+"""Why an ``agent investigate`` stream closed mid-run could not record its run.
+
+Raised from the stream's close, the error would reach only treaty, which
+prints its traceback and still exits 0; kept here instead, ``cloudfall.cli``
+reports it on stderr and exits ``PRECONDITION``, as a run whose record could
+not be written does when its stream ran to the end.
+"""
 
 
 @dataclass(frozen=True, slots=True, kw_only=True)
@@ -2424,7 +2432,12 @@ def agent_investigate(
         if stopped is None:
             message = "the investigation ended before its reader went away"
             raise RuntimeError(message) from None
-        _save_investigation(store, stopped)
+        try:
+            store.save(stopped)
+        except InvestigationError as error:
+            # The record is lost; the run still ends non-zero, PRECONDITION.
+            stream_unrecorded.append(error)
+            raise GeneratorExit from error
         stream_stopped.set()
         raise
     investigation = _save_investigation(store, investigation)

sdk/src/cloudfall/cli.py

@@ -2,10 +2,18 @@
 
 from __future__ import annotations
 
+import json
 import sys
 from typing import TYPE_CHECKING
 
-from cloudfall.app import INVESTIGATION_STOPPED, app, stream_stopped
+from treaty import FrameworkCode
+
+from cloudfall.app import (
+    INVESTIGATION_STOPPED,
+    app,
+    stream_stopped,
+    stream_unrecorded,
+)
 
 if TYPE_CHECKING:
     from collections.abc import Sequence
@@ -16,25 +24,47 @@
 
 def main(argv: Sequence[str] | None = None) -> int:
     """Run one command in-process and return its exit code."""
-    stream_stopped.clear()
+    _forget_streams()
     return _stopped(app.run(list(sys.argv[1:] if argv is None else argv)))
 
 
 def run() -> None:
     """Installed console-script entry point."""
-    stream_stopped.clear()
+    _forget_streams()
     try:
         app.main()
     except SystemExit as exiting:
         # app.main() exits with treaty's code; only a stopped stream's changes.
         code = exiting.code
-        if isinstance(code, int) and _stopped(code) != code:
-            raise SystemExit(INVESTIGATION_STOPPED) from None
+        if isinstance(code, int):
+            stopped = _stopped(code)
+            if stopped != code:
+                raise SystemExit(stopped) from None
         raise
 
 
+def _forget_streams() -> None:
+    stream_stopped.clear()
+    stream_unrecorded.clear()
+
+
 def _stopped(code: int) -> int:
-    """Return INVESTIGATION_STOPPED when the reader left an investigation mid-run."""
-    if stream_stopped.is_set() and code in OUTPUT_CLOSED:
+    """Return the exit code of an investigation whose reader left it mid-run.
+
+    INVESTIGATION_STOPPED when its record was kept; PRECONDITION, with the
+    reason on stderr, when the record could not be written.
+    """
+    if code not in OUTPUT_CLOSED:
+        return code
+    if stream_unrecorded:
+        error = stream_unrecorded[-1]
+        line = {
+            "code": FrameworkCode.PRECONDITION.name,
+            "message": error.detail,
+            "context": {"code": error.code},
+        }
+        sys.stderr.write(f"{json.dumps(line)}\n")
+        return int(FrameworkCode.PRECONDITION)
+    if stream_stopped.is_set():
         return INVESTIGATION_STOPPED
     return code

sdk/src/cloudfall/investigation.py

@@ -56,6 +56,7 @@
 ERROR_MODEL_REFUSED = "agent_model_refused"
 ERROR_MODEL_UNREACHABLE = "agent_model_unreachable"
 ERROR_INVESTIGATION_EXISTS = "agent_investigation_exists"
+ERROR_INVESTIGATIONS_UNWRITABLE = "agent_investigations_unwritable"
 
 SYSTEM_PROMPT = """\
 You are the on-call agent for a server fleet. You act only through the tools, \
@@ -321,9 +322,14 @@ def save(self, investigation: Investigation) -> Investigation:
         """Persist a new investigation and return it as saved.
 
         An investigation started in the same second as a recorded one gets a
-        numeric suffix on its id; no record is ever overwritten.
+        numeric suffix on its id; no record is ever overwritten. A directory
+        that cannot be written, such as a file in its place, raises
+        ``InvestigationError`` naming it.
         """
-        self.directory.mkdir(parents=True, exist_ok=True)
+        try:
+            self.directory.mkdir(parents=True, exist_ok=True)
+        except OSError as error:
+            raise self._unwritable(error) from error
         base = investigation.investigation_id.value
         for attempt in range(1, SAVE_ATTEMPTS + 1):
             identifier = base if attempt == 1 else f"{base}-{attempt}"
@@ -338,10 +344,19 @@ def save(self, investigation: Investigation) -> Investigation:
                     record.write(f"{json.dumps(document, indent=2, sort_keys=True)}\n")
             except FileExistsError:
                 continue
+            except OSError as error:
+                raise self._unwritable(error) from error
             return saved
         message = f"{SAVE_ATTEMPTS} investigations already exist for {base}"
         raise InvestigationError(ERROR_INVESTIGATION_EXISTS, message)
 
+    def _unwritable(self, error: OSError) -> InvestigationError:
+        reason = error.strerror or type(error).__name__
+        message = (
+            f"cannot write the investigation record to {self.directory}: {reason}"
+        )
+        return InvestigationError(ERROR_INVESTIGATIONS_UNWRITABLE, message)
+
 
 @dataclass(frozen=True, slots=True)
 class ToolCallText:

sdk/tests/test_investigation.py

@@ -15,6 +15,7 @@
 from cloudfall.app import app
 from cloudfall.cli import main
 from cloudfall.investigation import (
+    ERROR_INVESTIGATIONS_UNWRITABLE,
     ERROR_MODEL_REFUSED,
     TRACE_TEXT_LIMIT,
     AgentTool,
@@ -1191,10 +1192,13 @@ def log_message(self, *_args: object) -> None:
         return
 
 
-def test_a_reader_closing_stdout_mid_run_leaves_the_record_stopped(
-    tmp_path: Path,
-) -> None:
-    repository = _repository(tmp_path)
+def _closed_mid_run(
+    tmp_path: Path, repository: Path
+) -> tuple[dict[str, Any], dict[str, Any], int, str]:
+    """Run the CLI, read the first turn and its call, then close stdout.
+
+    Return the two events read, the exit code and what went to stderr.
+    """
     (repository / "playbooks" / "facts.yml").write_text(
         "---\n- hosts: localhost\n  connection: local\n  gather_facts: false\n"
         "  tasks:\n    - name: Report\n      ansible.builtin.debug:\n"
@@ -1246,13 +1250,21 @@ def test_a_reader_closing_stdout_mid_run_leaves_the_record_stopped(
     finally:
         server.shutdown()
         server.server_close()
+    return first, call, exit_code, stderr_path.read_text(encoding="utf-8")
+
+
+def test_a_reader_closing_stdout_mid_run_leaves_the_record_stopped(
+    tmp_path: Path,
+) -> None:
+    repository = _repository(tmp_path)
+
+    first, call, exit_code, err = _closed_mid_run(tmp_path, repository)
 
     assert (first["kind"], call["kind"]) == ("turn", "call")
     assert exit_code == 94
     # Nothing more is written once stdout is gone: no traceback, and no
     # "Exception ignored ... BrokenPipeError" at shutdown either. stderr holds
     # only the stream's JSON warning lines, such as UNTRUSTED_CONTENT.
-    err = stderr_path.read_text(encoding="utf-8")
     assert "BrokenPipe" not in err
     assert "Traceback" not in err
     assert all("code" in json.loads(line) for line in err.splitlines())
@@ -1265,3 +1277,67 @@ def test_a_reader_closing_stdout_mid_run_leaves_the_record_stopped(
     [step] = spec["steps"]
     assert step["outcome"] == "ran"
     assert list((repository / "decisions").glob(f"{step['decision']}*"))
+
+
+def test_an_unwritable_investigations_folder_is_refused_naming_it(
+    tmp_path: Path,
+) -> None:
+    report, rotate = _tools()
+    done = investigate(
+        "alert",
+        _tool_list(report, rotate),
+        ENDPOINT,
+        _Script(_say("ROOT CAUSE: disk\nPROPOSED: none\nWHY: read only.")),
+    )
+    folder = tmp_path / "investigations"
+    folder.write_text("not a folder\n", encoding="utf-8")
+    store = InvestigationStore(
+        directory=folder, catalog=SchemaCatalog(default_schema_directory())
+    )
+
+    with pytest.raises(InvestigationError) as error:
+        store.save(done)
+
+    assert error.value.code == ERROR_INVESTIGATIONS_UNWRITABLE
+    assert str(folder) in error.value.detail
+
+
+def test_a_record_that_cannot_be_written_exits_precondition(
+    tmp_path: Path, capsys: pytest.CaptureFixture[str], model_url: str
+) -> None:
+    repository = _repository(tmp_path)
+    (repository / "investigations").write_text("not a folder\n", encoding="utf-8")
+    key = tmp_path / "model.key"
+    key.write_text("sk-test\n", encoding="utf-8")
+    _Model.replies[:] = [
+        _say("ROOT CAUSE: /var/log/shop\nPROPOSED: none\nWHY: read only.")
+    ]
+
+    exit_code = _investigate_against(repository, key, model_url)
+
+    out = capsys.readouterr()
+    payload = _streamed(out.out)
+    assert exit_code == 4, payload
+    assert payload["error"]["code"] == "PRECONDITION"
+    assert payload["error"]["context"]["code"] == ERROR_INVESTIGATIONS_UNWRITABLE
+    assert str(repository / "investigations") in payload["error"]["message"]
+    assert "Traceback" not in out.err
+
+
+def test_a_stopped_run_whose_record_cannot_be_written_exits_precondition(
+    tmp_path: Path,
+) -> None:
+    repository = _repository(tmp_path)
+    (repository / "investigations").write_text("not a folder\n", encoding="utf-8")
+
+    first, call, exit_code, err = _closed_mid_run(tmp_path, repository)
+
+    assert (first["kind"], call["kind"]) == ("turn", "call")
+    # Not 0, as when the save crashed, and not 94: no stopped record was kept.
+    assert exit_code == 4, err
+    assert "Traceback" not in err
+    lines = [json.loads(line) for line in err.splitlines()]
+    [refused] = [line for line in lines if line["code"] == "PRECONDITION"]
+    assert refused["context"]["code"] == ERROR_INVESTIGATIONS_UNWRITABLE
+    assert str(repository / "investigations") in refused["message"]
+    assert (repository / "investigations").is_file()