diff --git a/docs/Development-Plan.md b/docs/Development-Plan.md index 2ba232f..5f85132 100644 --- a/docs/Development-Plan.md +++ b/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 diff --git a/docs/Tuning-Guide.md b/docs/Tuning-Guide.md index dd394de..dd0de1c 100644 --- a/docs/Tuning-Guide.md +++ b/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. diff --git a/nisse-dev.example.yaml b/nisse-dev.example.yaml index 0574027..ba612e7 100644 --- a/nisse-dev.example.yaml +++ b/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) diff --git a/nisse-dev/src/nisse_dev/loop/agent.py b/nisse-dev/src/nisse_dev/loop/agent.py index 8cee2a2..32abf62 100644 --- a/nisse-dev/src/nisse_dev/loop/agent.py +++ b/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) - last = preflight.tools_of((marker or {}).get("toolchain")) + 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")) + mode = preflight.SETUP.format( + last=( + f"Last time, this project's toolchain was: {preflight.said(last)}." + if last + else "" + ) + ) system = preflight.SYSTEM.format( - repo=project.repo, - workspace=workspace.describe(), - last=( - f"Last time, this project's toolchain was: {preflight.said(last)}. " - "Set that up again, then check whether it needs anything else.\n" - if last - else "" - ), + repo=project.repo, workspace=workspace.describe(), mode=mode ) - self._fact(item, "preflight: getting the project's toolchain ready") + 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) - self._active_thread = loop.thread - self._active_name = self._label(item) - self._running = True + 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: - self._running = False - if outcome.kind == "stopped": + if not outer: + self._running = False + 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: ( diff --git a/nisse-dev/src/nisse_dev/loop/preflight.py b/nisse-dev/src/nisse_dev/loop/preflight.py index f512f85..d4e6ca1 100644 --- a/nisse-dev/src/nisse_dev/loop/preflight.py +++ b/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 = [ diff --git a/nisse-dev/src/nisse_dev/loop/tools.py b/nisse-dev/src/nisse_dev/loop/tools.py index 298dd9a..bcef23a 100644 --- a/nisse-dev/src/nisse_dev/loop/tools.py +++ b/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", diff --git a/nisse-dev/src/nisse_dev/state.py b/nisse-dev/src/nisse_dev/state.py index 382f057..6405558 100644 --- a/nisse-dev/src/nisse_dev/state.py +++ b/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 } ) diff --git a/nisse-dev/src/nisse_dev/views.py b/nisse-dev/src/nisse_dev/views.py index 4b253f0..c8156d2 100644 --- a/nisse-dev/src/nisse_dev/views.py +++ b/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"}, } diff --git a/nisse-dev/tests/test_loop_properties.py b/nisse-dev/tests/test_loop_properties.py index fb81552..b921e22 100644 --- a/nisse-dev/tests/test_loop_properties.py +++ b/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] diff --git a/nisse-dev/tests/test_loop_preflight.py b/nisse-dev/tests/test_loop_toolchain.py similarity index 66% rename from nisse-dev/tests/test_loop_preflight.py rename to nisse-dev/tests/test_loop_toolchain.py index 51ae40e..f0d2f1d 100644 --- a/nisse-dev/tests/test_loop_preflight.py +++ b/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