Merge pull request 'M4.2: the toolchain step' (#136) from toolchain-step into main

docs/Development-Plan.md

@@ -402,7 +402,8 @@ needs; code makes sure that happens, bounds it and checks the result.
   without it), preflight runs on the driver's model, gaps don't block. Turning
   off unattended upgrades and the template's baseline are template choices.
 
-**M4.2** *(revised with the operator: in place of recording installs).* The
+**M4.2** *(revised with the operator: in place of recording installs).*
+**Done** (this PR). The
 preflight becomes **the toolchain step** (D3): make the VM and the project's own
 files agree, with the repository as the record rather than a harness-kept one.
 - Started by the harness when the VM isn't set up for the project (M4.1's

docs/Tuning-Guide.md

@@ -195,9 +195,10 @@ looking around, not doing the work. Before M2.8 this was
 
 ### `tuning.preflight.max_turns` — default 20
 
-How many model calls the toolchain preflight may make (D3). The preflight runs
-before the driver plans, whenever a project's build VM has no marker from a
-finished one: a new project, a new or wiped VM.
+How many model calls one toolchain step may make (D3). The harness starts one
+before the driver plans whenever a project's build VM has no marker from a
+finished one (a new project, a new or wiped VM), and the driver asks for one
+when the work needs a toolchain change.
 
 - **Too low:** a project with several things to install doesn't finish, and
   says so; work goes on with what's missing stated, and the next item tries
@@ -209,7 +210,7 @@ finished one: a new project, a new or wiped VM.
 
 ### `tuning.preflight.command_timeout_s` — default 900
 
-The limit on one command during the preflight, in place of
+The limit on one command during a toolchain step, in place of
 `tuning.tools.command_timeout_s`: installing a compiler or a language runtime
 can take minutes. Commands in ordinary runs keep their own, shorter limit.
 

nisse-dev.example.yaml

@@ -90,8 +90,8 @@ tuning:
   driver:
     max_turns: 40            # model calls per driver run before it stops as capped
     failure_limit: 2         # consecutive failed commands before escalating (L9)
-  preflight:                 # getting a project's toolchain ready (D3)
-    max_turns: 20            # model calls the preflight run may make
+  preflight:                 # the toolchain step (D3)
+    max_turns: 20            # model calls one toolchain step may make
     command_timeout_s: 900   # one command in it: installs take a while
   expert:
     consult_budget: 3        # ask_expert calls per milestone (L9)

nisse-dev/src/nisse_dev/loop/agent.py

@@ -26,7 +26,14 @@ from dataclasses import dataclass, replace
 from datetime import UTC, datetime
 from typing import Any
 
-from nisse_core.actions import DANGEROUS, REFUSED, UNKNOWN, Action, Verdict
+from nisse_core.actions import (
+    CONTAINED,
+    DANGEROUS,
+    REFUSED,
+    UNKNOWN,
+    Action,
+    Verdict,
+)
 from nisse_dev.cancel import Cancel
 from nisse_dev.canonical import (
     ActionVerdict,
@@ -79,6 +86,7 @@ from nisse_dev.loop.tools import (
     REPLY,
     RESTATE,
     START_MILESTONE,
+    TOOLCHAIN_STEP,
     WAIT_FOR_REVIEW,
 )
 from nisse_dev.oplog import OpLog
@@ -148,10 +156,10 @@ Commands that reach beyond this repository and its build VM (pushing, other
 hosts, uploads, publishing) or could break the host are put to the operator
 before they run; if they allow one, run it again exactly as before.
 
-The build VM is the project's: if the work needs a tool or library, install
-it there and say why, and put lasting needs in the project's own pins
-(.python-version, rust-toolchain.toml, requires-python, engines) so they're
-reviewed with the work.
+If the work needs a toolchain change (a new tool, library or version), ask for
+a toolchain_step with why: it installs it and keeps the project's pins, setup
+docs and CI configuration in step, as part of the work. Before the go-ahead,
+you only look: say what the work needs in your restatement.
 Never change branches yourself.
 
 Before each group of tool calls, say in a sentence what you're doing and why:
@@ -514,7 +522,8 @@ class LoopAgent:
         versions checked again, and a change said."""
         assert project.workspace is not None
         if marker is None or (starting and not marker.get("complete")):
-            return self._preflight_run(item, project, loop, marker)
+            stepped = self._toolchain_step(item, project, loop, marker)
+            return None if stepped is None else stepped[0]
         host = project.workspace.host
         before = preflight.tools_of(loop.toolchain) or preflight.tools_of(
             marker.get("toolchain")
@@ -532,47 +541,72 @@ class LoopAgent:
             self._fact(item, f"toolchain: {preflight.said(now)}")
         return replace(loop, toolchain=tuple(tuple(t.to_data()) for t in now))
 
-    def _preflight_run(
+    def _toolchain_step(
         self,
         item: WorkItem,
         project: Project,
         loop: Loop,
         marker: dict[str, Any] | None,
-    ) -> Loop | None:
-        """D3: before any planning, the item's driver gets the project's
-        toolchain ready; the harness checks what it reports."""
+        *,
+        reason: str = "",
+    ) -> tuple[Loop, str] | None:
+        """D3: the toolchain step, on the item's driver model in a transcript
+        of its own. Without a ``reason`` the harness started it (the VM isn't
+        set up): it sets the VM up from the repository and edits nothing
+        there. With one the driver asked: it may bring the project's files in
+        step. The harness checks what it reports. None when it was stopped
+        before the work (settled)."""
         assert project.workspace is not None
         workspace = project.workspace.with_timeout(self._preflight_timeout_s)
+        requested = bool(reason)
+        if requested:
+            last = preflight.tools_of(loop.toolchain)
+            mode = preflight.REQUESTED.format(reason=reason)
+        else:
             last = preflight.tools_of((marker or {}).get("toolchain"))
-        system = preflight.SYSTEM.format(
-            repo=project.repo,
-            workspace=workspace.describe(),
+            mode = preflight.SETUP.format(
                 last=(
-                f"Last time, this project's toolchain was: {preflight.said(last)}. "
-                "Set that up again, then check whether it needs anything else.\n"
+                    f"Last time, this project's toolchain was: {preflight.said(last)}."
                     if last
                     else ""
-            ),
                 )
-        self._fact(item, "preflight: getting the project's toolchain ready")
+            )
+        system = preflight.SYSTEM.format(
+            repo=project.repo, workspace=workspace.describe(), mode=mode
+        )
+        self._fact(
+            item,
+            f"toolchain step requested: {reason}"
+            if requested
+            else "toolchain step: setting the build VM up from the repository",
+        )
+        before = self._changed_files(project)
         roles = self._roles_of(item)
+        outer = self._running  # a requested step runs inside the driver's run
+        if not outer:
             self._active_thread = loop.thread
             self._active_name = self._label(item)
             self._running = True
         try:
             outcome = self._driver.run(
-                self._store.transcript(item.id, "driver"),
+                self._store.transcript(item.id, "toolchain"),
                 system=system,
-                tools=[*EXECUTION_TOOLS, preflight.TOOLCHAIN_READY],
+                tools=[
+                    *(EXECUTION_TOOLS if requested else _LOOK),
+                    preflight.TOOLCHAIN_READY,
+                ],
                 workspace=workspace,
                 checks={"toolchain_ready": preflight.report_problem},
                 model=roles.resolve("driver"),
-                screen=lambda call: self._screen(item.id, str(workspace.root), call),
+                screen=lambda call: self._screen(
+                    item.id, str(workspace.root), call, guard=False
+                ),
                 max_turns=self._preflight_turns,
             )
         finally:
+            if not outer:
                 self._running = False
-        if outcome.kind == "stopped":
+        if outcome.kind == "stopped" and not outer:
             self._cancel.clear()
             self._settle(item.id, outcome)
             return None
@@ -586,7 +620,7 @@ class LoopAgent:
             complete = True
         else:
             report = preflight.Report(
-                f"the preflight didn't finish ({outcome.note or outcome.kind})", last
+                f"the step didn't finish ({outcome.note or outcome.kind})", last
             )
             installed, complete = [], False
         try:
@@ -610,13 +644,59 @@ class LoopAgent:
             parts.append("installed: " + ", ".join(installed))
         if missing:
             parts.append("missing: " + "; ".join(missing))
-        if not complete:
+        if not complete or requested:
             parts.append(report.summary)
-        said = "; ".join(parts)
-        self._fact(item, f"preflight: {said}")
+        if not requested:
+            # D3: the harness's step edits nothing in the repository.
+            edited = sorted(set(self._changed_files(project)) - set(before))
+            if edited:
+                parts.append(
+                    "it changed files it shouldn't have, left for the work: "
+                    + ", ".join(edited)
+                )
+        said = "; ".join(p for p in parts if p)
+        self._fact(item, f"toolchain step: {said}")
         if installed or missing or not complete:
-            self._transport.post(f"Preflight: {said}.", thread=loop.thread)
-        return replace(now, toolchain=tuple(tuple(t.to_data()) for t in tools))
+            self._transport.post(f"Toolchain step: {said}.", thread=loop.thread)
+        loop = replace(now, toolchain=tuple(tuple(t.to_data()) for t in tools))
+        return loop, said
+
+    def _changed_files(self, project: Project) -> list[str]:
+        """The workspace's changes, as git sees them (for the setup step's
+        promise to edit nothing)."""
+        assert project.workspace is not None
+        try:
+            result = project.workspace.host.execute(
+                "git status --porcelain", cwd=str(project.workspace.root), timeout_s=30
+            )
+        except HostUnavailable:
+            return []
+        return [line[3:] for line in result.stdout.splitlines() if line.strip()]
+
+    def _requested_step(self, item_id: str, reason: str) -> tuple[str, bool]:
+        """The driver's toolchain_step: once the work is agreed (D1)."""
+        item = self._store.work_item(item_id)
+        assert item is not None
+        loop = loop_of(item)
+        project = self._at(item)
+        if not loop.started:
+            return (
+                (
+                    "not before the go-ahead: say what the work needs in your "
+                    "restatement, and ask for the step once work has started"
+                ),
+                False,
+            )
+        if not reason:
+            return "say what the work needs, and why", False
+        if project.workspace is None:
+            return "there's no build VM here", False
+        stepped = self._toolchain_step(item, project, loop, None, reason=reason)
+        if stepped is None:
+            return "the toolchain step was stopped", False
+        loop, said = stepped
+        save(self._store, item.id, item.position, loop, cause="toolchain step")
+        return f"toolchain step done: {said}", True
 
     # -- the trust dial (L3, D5; M3.3) --------------------------------------------
 
@@ -1904,7 +1984,9 @@ class LoopAgent:
 
     # -- the fast path (P4, D6) -------------------------------------------------
 
-    def _screen(self, item_id: str, root: str, call: ToolCall) -> Screening:
+    def _screen(
+        self, item_id: str, root: str, call: ToolCall, *, guard: bool = True
+    ) -> Screening:
         """The driver's execution call, judged before it runs (P4): by the
         fast path's rules, then the classifier for what they don't know
         (L4). A dangerous one runs only when the operator allowed exactly
@@ -1915,8 +1997,24 @@ class LoopAgent:
         action = Action(call.tool, dict(call.arguments))
         key = action.key()
         verdict, judged = self._judge(item, "driver", call, action, root)
+        early = (
+            guard
+            and not loop.started
+            and (
+                verdict.effect == DANGEROUS
+                or (judged is None and verdict.effect == CONTAINED)
+            )
+        )
         if verdict.effect == REFUSED:
             decision, screening = "refused", Screening("refuse", verdict.reason)
+        elif early:
+            # D1: until the go-ahead the agent only looks.
+            decision = "refused"
+            screening = Screening(
+                "refuse",
+                f"not before the go-ahead: {verdict.reason}. Say what the work "
+                "needs in your restatement",
+            )
         elif judged is not None and verdict.rule == IN_SCOPE:
             decision, screening = "ran", Screening("run")
             self._in_scope(item, action, verdict.reason)
@@ -2336,6 +2434,7 @@ class LoopAgent:
                     CLOSE_ITEM,
                     *([WAIT_FOR_REVIEW] if loop.pr else []),
                     *([DELEGATE, ASK_EXPERT] if self._helpers else []),
+                    *([TOOLCHAIN_STEP] if checked and project.workspace else []),
                     *([REPLY] if loop.back_to else []),
                     *EXECUTION_TOOLS,
                 ]
@@ -2376,6 +2475,9 @@ class LoopAgent:
                     "ask_expert": lambda args: self._ask_expert(
                         item.id, str(args.get("question", "")).strip()
                     ),
+                    "toolchain_step": lambda args: self._requested_step(
+                        item.id, str(args.get("reason", "")).strip()
+                    ),
                 },
                 checks={
                     "restate": lambda args: (

nisse-dev/src/nisse_dev/loop/preflight.py

@@ -229,15 +229,16 @@ TOOLCHAIN_READY = {
     },
 }
 
-SYSTEM = """You are nisse-dev, getting a project's toolchain ready on its build VM
-before any work starts. The project is {repo}; its clone is the workspace.
+SYSTEM = """You are nisse-dev, running a toolchain step for {repo}: making its
+build VM and the project's own files agree. Its clone is the workspace.
 {workspace}
 
-Read the project's own files to work out what it needs to build, test and run:
-its language runtimes and compilers, package and build tools, and anything else
-it calls (system libraries, CLIs, a database for its tests). Honour its pins
+Work out from the project's own files what it needs to build, test and run: its
+language runtimes and compilers, package and build tools, and anything else it
+calls (system libraries, CLIs, a database for its tests). Honour its pins
 (.python-version, rust-toolchain.toml, requires-python, engines, lockfiles).
-{last}
+
+{mode}
 Check what's there with version commands; install what's missing. The build
 VM is the project's and disposable: installing on it is fine. Don't install or
 change anything for other hosts, and never install, store or ask for
@@ -252,6 +253,19 @@ found and did: each tool whose version matters with a command that prints its
 version, what you installed, and what's still missing and why. Keep it short:
 this is setup, not the work."""
 
+SETUP = """This build VM is new to the project, or was rebuilt. Set it up from what
+the repository says: its pins, setup docs and CI configuration. Don't change
+the repository's files: if they're out of step with what the project needs,
+say so in your summary, and the work can put them right.
+{last}"""
+
+REQUESTED = """The work needs a toolchain change: {reason}
+Install what it needs, then bring the project's own files in step with it: its
+pins (.python-version, rust-toolchain.toml, requires-python, engines,
+lockfiles), its setup docs (the README, or wherever it documents setup) and its
+CI configuration. Those edits are part of the current work and are reviewed
+with it."""
+
 
 def report_of(arguments: dict[str, Any]) -> Report:
     tools = [

nisse-dev/src/nisse_dev/loop/tools.py

@@ -69,6 +69,15 @@ WAIT_FOR_REVIEW = _tool(
     note="What you're waiting on, for the operator.",
 )
 
+TOOLCHAIN_STEP = _tool(
+    "toolchain_step",
+    "The work needs a toolchain change (a new tool, library or version): a "
+    "step that installs it on the build VM and brings the project's pins, "
+    "setup docs and CI configuration in step. Its edits are part of the "
+    "current work. Returns its report.",
+    reason="What the work needs, and why.",
+)
+
 # A conversation's tools (L1, L5): it talks, and may propose work.
 REPLY = _tool(
     "reply",

nisse-dev/src/nisse_dev/state.py

@@ -54,6 +54,7 @@ ACTORS = frozenset(
         "helper",
         "expert",
         "classifier",
+        "toolchain",  # the toolchain step (D3): the driver's model, apart
     }
 )
 

nisse-dev/src/nisse_dev/views.py

@@ -41,6 +41,9 @@ NARRATION_RESULT_CHARS = 300
 _SHOWN: dict[str, dict[str, str]] = {
     "driver": {"helper": "report"},
     "helper": {"driver": "narrate"},
+    # The toolchain step works in its own transcript: the driver sees only
+    # its report (a tool result, or facts), the step the driver's work.
+    "toolchain": {"driver": "narrate"},
 }
 
 

nisse-dev/tests/test_loop_properties.py

@@ -145,6 +145,12 @@ class PolicyModel:
             return [call("ask_human", question=f"talk first {self.n}?")]  # discuss
         if "finish_milestone" not in offered:
             return [call("restate", text=f"restatement {self.n}", changed=f"c{self.n}")]
+        if "toolchain_step" in offered and self.rng.random() < 0.06:
+            # M4.2: the work asks for a toolchain change.
+            return [
+                call("start_milestone", name=f"m{self.n}"),
+                call("toolchain_step", reason=f"needs a tool {self.n}"),
+            ]
         if "delegate" in offered and self.rng.random() < 0.1:
             return [call("delegate", task=f"help {self.n}")]
         allowed = [t for t in facts[-3:] if "allowed this once" in t]

nisse-dev/tests/test_loop_toolchain.py

@@ -1,8 +1,9 @@
-"""Toolchain preflight in the loop (D3, M4.1): before the driver plans, the
-build VM is checked (baseline, clone, marker); without a marker the item's
-driver gets the toolchain ready and the harness checks what it reports; later
-runs check the versions again; a rebuilt VM is re-cloned and set up again,
-and a changed host key waits for the operator."""
+"""The toolchain step in the loop (D3; M4.1, M4.2): before the driver plans,
+the build VM is checked (baseline, clone, marker); without a marker the harness
+starts a toolchain step that sets the VM up from the repository and edits
+nothing there; the driver can ask for one once work is agreed, whose edits are
+part of the work; later runs check versions again; a rebuilt VM is re-cloned
+and set up again, and a changed host key waits for the operator."""
 
 import io
 import shutil
@@ -102,7 +103,9 @@ def said(transport: InMemoryTransport) -> list[str]:
     return [m.text for m in transport.outbound]
 
 
-def test_preflight_comes_before_planning(world: dict[str, Any], project: str) -> None:
+def test_the_setup_step_comes_before_planning(
+    world: dict[str, Any], project: str
+) -> None:
     model = ScriptedModel(ready(), RESTATE)
     agent, transport = agent_for(world, project, model)
     agent.intake()
@@ -110,21 +113,21 @@ def test_preflight_comes_before_planning(world: dict[str, Any], project: str) ->
     assert (item.position, loop_of(item).wait) == (WAITING, GO_AHEAD)
     known = facts(world)
     toolchain = [
-        f for f in known if f.startswith("preflight: toolchain: python Python 3.")
+        f for f in known if f.startswith("toolchain step: toolchain: python Python 3.")
     ]
     assert toolchain, known
     assert known.index(
-        "preflight: getting the project's toolchain ready"
+        "toolchain step: setting the build VM up from the repository"
     ) < known.index("restatement rev-1 put up for a go-ahead")
     assert loop_of(item).toolchain[0][:2] == ("python", "python3 --version")
     marker = preflight.check(
         world["host"], project, str(world["repo"].root), markers(world)
     ).marker
     assert marker is not None and marker["complete"]
-    assert not [t for t in said(transport) if t.startswith("Preflight:")]  # quiet
+    assert not [t for t in said(transport) if t.startswith("Toolchain step:")]  # quiet
 
 
-def test_preflight_runs_once_then_versions_are_checked(
+def test_the_setup_step_runs_once_then_versions_are_checked(
     world: dict[str, Any], project: str, monkeypatch: pytest.MonkeyPatch
 ) -> None:
     model = ScriptedModel(ready(), RESTATE, [*WORK, *FINISH])
@@ -179,12 +182,12 @@ def test_what_was_installed_or_is_missing_is_said(
     )
     agent, transport = agent_for(world, project, model)
     agent.intake()
-    [line] = [t for t in said(transport) if t.startswith("Preflight:")]
+    [line] = [t for t in said(transport) if t.startswith("Toolchain step:")]
     assert "installed: libfoo-dev" in line
     assert "missing: protoc (no package for this OS)" in line
 
 
-def test_an_unfinished_preflight_is_said_and_work_goes_on(
+def test_an_unfinished_step_is_said_and_work_goes_on(
     world: dict[str, Any], project: str
 ) -> None:
     from nisse_dev.canonical import AssistantText
@@ -194,7 +197,7 @@ def test_an_unfinished_preflight_is_said_and_work_goes_on(
     agent, transport = agent_for(world, project, model)
     agent.intake()
     assert loop_of(the_item(world)).wait == GO_AHEAD  # it planned anyway
-    assert any("the preflight didn't finish" in t for t in said(transport))
+    assert any("the step didn't finish" in t for t in said(transport))
     marker = preflight.check(
         world["host"], project, str(world["repo"].root), markers(world)
     ).marker
@@ -213,7 +216,10 @@ def test_a_wiped_vm_is_cloned_again_and_set_up(
     say(transport, world, "go")
     assert any("cloned the workspace again" in f for f in facts(world))
     assert (Path(world["repo"].root) / ".git").exists()
-    assert sum("getting the project's toolchain ready" in f for f in facts(world)) == 2
+    assert (
+        sum("setting the build VM up from the repository" in f for f in facts(world))
+        == 2
+    )
     assert world["forge"].prs  # and the work went on to a PR
 
 
@@ -254,3 +260,81 @@ def test_a_changed_host_key_waits_for_the_operators_word(
     say(transport, world, "trust new host key")
     assert forgotten == ["yes"]
     assert loop_of(the_item(world)).wait == GO_AHEAD  # set up, then planned
+
+
+# --- M4.2: the step's modes, and the go-ahead ----------------------------------------
+
+
+def test_the_setup_step_edits_nothing_in_the_repository(
+    world: dict[str, Any], project: str
+) -> None:
+    step = [
+        call("run_command", command="echo stray > stray.txt"),
+        *ready(),
+    ]
+    agent, _ = agent_for(world, project, ScriptedModel(step, RESTATE))
+    agent.intake()
+    [said] = [f for f in facts(world) if f.startswith("toolchain step: toolchain")]
+    assert "changed files it shouldn't have, left for the work: stray.txt" in said
+    tools = [
+        e.event.tool
+        for e in world["store"].transcript(the_item(world).id, "toolchain").read()
+        if e.actor == "toolchain" and hasattr(e.event, "tool")
+    ]
+    assert "write_file" not in tools  # not offered in setup
+
+
+def test_a_requested_step_brings_the_projects_files_in_step(
+    world: dict[str, Any], project: str
+) -> None:
+    requested = [
+        call("write_file", path="README.md", content="# demo\n\nNeeds Python 3.12.\n"),
+        *ready(installed=["httpx"]),  # type: ignore[arg-type]
+    ]
+    model = ScriptedModel(
+        ready(),
+        RESTATE,
+        [
+            call("start_milestone", name="m"),
+            call("toolchain_step", reason="it needs httpx, and the README says so"),
+        ],
+        requested,
+        [
+            call("write_file", path="w.py", content="import httpx\n"),
+            call("finish_milestone", summary="uses httpx", final="yes"),
+        ],
+    )
+    agent, transport = agent_for(world, project, model)
+    agent.intake()
+    say(transport, world, "go")
+    forge = world["forge"]
+    [pr] = [n for n, p in forge.prs.items() if not p["head"].startswith("nisse-check")]
+    assert sorted(forge.files(pr)) == ["README.md", "w.py"]  # reviewed with the work
+    driver = [
+        e.event for e in world["store"].transcript(the_item(world).id, "driver").view()
+    ]
+    results = [getattr(e, "result", "") for e in driver]
+    assert any(r.startswith("toolchain step done: toolchain: python") for r in results)
+    # The step's own turns stay in its transcript.
+    calls = [getattr(e, "arguments", {}).get("path") for e in driver]
+    assert "README.md" not in calls
+
+
+def test_before_the_go_ahead_the_driver_only_looks(
+    world: dict[str, Any], project: str
+) -> None:
+    looking = [
+        call("run_command", command="python3 --version"),
+        call("run_command", command="pip install httpx"),
+    ]
+    restate = [call("restate", text="add count_words to text.py; it needs httpx")]
+    agent, _ = agent_for(world, project, ScriptedModel(ready(), looking, restate))
+    agent.intake()
+    results = [
+        e.event.result
+        for e in world["store"].transcript(the_item(world).id, "driver").read()
+        if isinstance(e.event, ToolResult) and e.actor == "driver"
+    ]
+    assert "Python 3." in results[0]  # looking runs
+    assert results[1].startswith("not run: not before the go-ahead")
+    assert loop_of(the_item(world)).wait == GO_AHEAD