fix: validate the project before generate.py deletes notebooks/

autohands/generate.py

@@ -59,6 +59,23 @@ def copy_to_notebooks(source):
 
 
 if __name__ == "__main__":
+    # Validate the project BEFORE anything destructive runs. The rmtree below
+    # clears the whole `notebooks/` tree, while `inject_colab_setup` only
+    # rejects an unknown project deep inside the per-script loop that follows
+    # it — so running this on an unregistered project deleted 113 tracked
+    # notebooks in autocti_workspace and *then* aborted. A workspace with a
+    # root `start_here.py` happened to fail safely (that loop injects before
+    # the rmtree); one without it did not. `build_util.COLAB_PROJECTS` stays
+    # the single source of truth, and `inject_colab_setup`'s own raise remains
+    # the backstop for direct callers.
+    if project not in build_util.COLAB_PROJECTS:
+        sys.exit(
+            f"generate.py: unknown project '{project}' — add it to "
+            f"COLAB_PROJECTS in build_util.py and to the _PROJECTS registry in "
+            f"PyAutoNerves's autonerves/setup_colab.py. Known: "
+            f"{sorted(build_util.COLAB_PROJECTS)}. Nothing was modified."
+        )
+
     report = None
     if args.report_dir:
         from result_collector import RunReport
@@ -115,11 +132,17 @@ def copy_to_notebooks(source):
 
     for script_path in iter_script_paths(scripts_path):
         start = time.time()
+        # `py_to_notebook` writes its intermediate .ipynb beside the source
+        # script, and it is only removed after a successful copy — so any
+        # exception in between strands one inside `scripts/`, where it is not
+        # tracked and not obviously debris. Cleaned up in `finally` below.
+        source_path = None
         try:
             source_path = build_util.py_to_notebook(script_path)
             build_util.inject_colab_setup(source_path, project)
             copy_to_notebooks(source_path)
             os.remove(source_path)
+            source_path = None
             if report is not None:
                 from result_collector import ScriptResult, Status
                 report.results.append(ScriptResult(
@@ -138,6 +161,9 @@ def copy_to_notebooks(source):
                 ))
             else:
                 raise
+        finally:
+            if source_path is not None and os.path.exists(source_path):
+                os.remove(source_path)
 
     for read_me_path in scripts_path.rglob("*.rst"):
         copy_to_notebooks(read_me_path)

tests/test_generate_validates_project.py

@@ -0,0 +1,108 @@
+"""Tests that generate.py validates the project before destroying anything.
+
+`generate.py` clears the whole ``notebooks/`` tree, and until this was fixed the
+only rejection of an unknown project happened inside the per-script loop that
+runs *after* that rmtree — so running it on an unregistered project deleted 113
+tracked notebooks in `autocti_workspace` and then aborted. These tests pin the
+ordering, since the failure is invisible in a passing run.
+
+Driven as a subprocess: `generate.py` parses argv and does all its work at
+module level under ``__main__``, so it cannot be exercised by import.
+"""
+
+import os
+import subprocess
+import sys
+from pathlib import Path
+
+AUTOHANDS_DIR = Path(__file__).parent.parent / "autohands"
+GENERATE = AUTOHANDS_DIR / "generate.py"
+
+sys.path.insert(0, str(AUTOHANDS_DIR))
+
+import build_util  # noqa: E402
+
+
+def _make_workspace(root: Path) -> Path:
+    """A minimal workspace with a tracked notebooks/ tree, as a real git repo."""
+    (root / "scripts" / "imaging").mkdir(parents=True)
+    (root / "scripts" / "imaging" / "modeling.py").write_text(
+        '"""\nModeling\n========\n\nIntro prose.\n"""\n\nvalue = 1\n'
+    )
+
+    notebooks = root / "notebooks" / "imaging"
+    notebooks.mkdir(parents=True)
+    (notebooks / "modeling.ipynb").write_text('{"cells": [], "nbformat": 4}\n')
+    (root / "notebooks" / "README.rst").write_text("Existing index\n")
+
+    subprocess.run(["git", "init", "-q"], cwd=root, check=True)
+    subprocess.run(["git", "add", "-A"], cwd=root, check=True)
+    subprocess.run(
+        ["git", "-c", "user.email=t@t", "-c", "user.name=t", "commit", "-qm", "seed"],
+        cwd=root,
+        check=True,
+    )
+    return root
+
+
+def _run_generate(workspace: Path, project: str):
+    env = dict(os.environ, PYTHONPATH=str(AUTOHANDS_DIR))
+    return subprocess.run(
+        [sys.executable, str(GENERATE), project],
+        cwd=workspace,
+        capture_output=True,
+        text=True,
+        env=env,
+    )
+
+
+def test_unknown_project_exits_nonzero_without_touching_notebooks(tmp_path):
+    workspace = _make_workspace(tmp_path)
+
+    result = _run_generate(workspace, "autocti")
+
+    assert result.returncode != 0
+
+    # The tree is untouched — this is the regression. Before the fix, `git
+    # status` showed every notebook deleted.
+    status = subprocess.run(
+        ["git", "status", "--porcelain"],
+        cwd=workspace,
+        capture_output=True,
+        text=True,
+        check=True,
+    )
+    assert status.stdout == ""
+    assert (workspace / "notebooks" / "imaging" / "modeling.ipynb").exists()
+    assert (workspace / "notebooks" / "README.rst").exists()
+
+    # ... and no intermediate notebook is stranded beside a source script.
+    assert list(workspace.glob("scripts/**/*.ipynb")) == []
+
+
+def test_unknown_project_message_names_the_project_and_both_registries(tmp_path):
+    workspace = _make_workspace(tmp_path)
+
+    message = _run_generate(workspace, "autocti").stderr
+
+    assert "autocti" in message
+    assert "COLAB_PROJECTS" in message
+    assert "setup_colab" in message
+    assert "Nothing was modified" in message
+    # The known set is listed, so the reader does not have to go find it.
+    for project in build_util.COLAB_PROJECTS:
+        assert project in message
+
+
+def test_known_project_passes_validation_and_regenerates(tmp_path):
+    workspace = _make_workspace(tmp_path)
+
+    result = _run_generate(workspace, "autolens")
+
+    assert result.returncode == 0, result.stderr
+    assert "unknown project" not in result.stderr
+
+    notebook = workspace / "notebooks" / "imaging" / "modeling.ipynb"
+    assert notebook.exists()
+    assert "Intro prose." in notebook.read_text()
+    assert list(workspace.glob("scripts/**/*.ipynb")) == []