diff --git a/docs/Architecture.md b/docs/Architecture.md index 8a83361..eb92ba0 100644 --- a/docs/Architecture.md +++ b/docs/Architecture.md @@ -95,8 +95,9 @@ hard wall; the classifier is a filter, not a guarantee. ### P5. Reversibility first - Work happens on **branches**, never on what others depend on (D4). -- Work starts from a **fresh, clean workspace** (D3): a broken environment is - replaced, not diagnosed. +- Work starts from a **fresh, clean workspace** (D3). A broken environment is + fixed the usual ways, and replaced when that doesn't work: the agent may ask + for its build VM to be reimaged rather than diagnose it at length. - **What must survive lives on the agent host and is snapshotted** (X4), and snapshots are taken before risky changes where the platform allows. Tool VMs, build artifacts and caches are disposable and rebuilt rather than repaired (X1). @@ -703,6 +704,14 @@ reported but not asked about. Changes meant to last go into the repository's fil through a toolchain step; recurring ones go into the template (X4). Installing on any other host is dangerous. +**A broken build VM blocks its projects.** When the usual fixes don't help, the +driver asks for the VM to be reimaged from its template. The harness pushes the +work in progress first, then blocks the VM: nothing runs on any project on it, +across restarts, until the operator says `vm ready` (which also trusts its new +host key), and a reimaged VM is set up again by the toolchain step. A VM that +can't be reached blocks the same way (#137). The operator recovers it for now; +`nisse-ops` later (O5). + ### D4. Git, branches and PRs - **The harness owns branches;** plans and models never create or switch them. diff --git a/docs/Development-Plan.md b/docs/Development-Plan.md index 5f85132..7080c4e 100644 --- a/docs/Development-Plan.md +++ b/docs/Development-Plan.md @@ -415,6 +415,29 @@ files agree, with the repository as the record rather than a harness-kept one. - With it, D1's rule made structural: until the go-ahead the fast path refuses anything that isn't harmless, so needs go in the restatement. +**M4.3** *(revised with the operator: a broken VM blocks its projects).* +**Done** (this PR). Cleaning caches and reinstalling dependencies were already +contained and reported as the actions they are, so what is left is a VM broken +past the usual fixes (D3). +- The driver can `request_reimage(reason)`, before the go-ahead too (it asks, + it doesn't act). Its prompt says to try the usual fixes first, then ask rather + than go on diagnosing. +- The harness parks the work first (pushed to Gitea, as for a switch; a VM too + broken to push loses it, and the operator is told), then **blocks the build + VM**: a durable block per host, so every project on it waits and none of its + items is taken up, across restarts. Said once at top level with the projects + blocked and the command that clears it, and in each blocked item's thread. +- The operator's `vm ready ` (no name needed when only one is blocked) + lifts the block and trusts the VM's new host key. A reimaged VM has no marker, + so the toolchain step sets it up again before work; one said ready without a + reimage goes straight on. The item that found it goes first. +- `loop.qnt` gained the block (`blocked_vm_runs_nothing`); it found that work in + progress was lost on a reimage, hence parking first. The walk requests + reimages and recovers them, reimaged or not. +- Next: an unreachable VM blocks through the same mechanism, unblocked by a slow + background retry once it is back (#137). Later, nisse-ops reimages a VM itself + (M6.2, M7.6); a changed address means editing `build_host` and restarting. + ### Phase 5 — Memory **M5.1 The vault.** *Pre-approved.* The folder, per-agent mounts, control tools, @@ -473,6 +496,7 @@ vision; one local model for every agent; several forges (#26); a room per projec | #69 inline review comments | M2.1 | | #70 watch stall | M2.1 | | #77 conversational agent | M2.3 (questions), M2.7 | +| #137 block on an unreachable build VM | After M4.3, which built the block | ## 6. Implementation notes diff --git a/nisse-dev/src/nisse_dev/loop/agent.py b/nisse-dev/src/nisse_dev/loop/agent.py index 32abf62..63b1a06 100644 --- a/nisse-dev/src/nisse_dev/loop/agent.py +++ b/nisse-dev/src/nisse_dev/loop/agent.py @@ -84,6 +84,7 @@ from nisse_dev.loop.tools import ( PROPOSE_PROJECT, PROPOSE_WORK, REPLY, + REQUEST_REIMAGE, RESTATE, START_MILESTONE, TOOLCHAIN_STEP, @@ -109,6 +110,8 @@ _COMMITTED = ( "finish_milestone, which pushes the milestone's branch and opens or " "updates its PR. There is no PR until then." ) +# The operator's word that a blocked build VM is ready again (D3). +_VM_READY = re.compile(r"vm ready(?: ([a-z0-9][a-z0-9-]*))?") _NEEDS_ITEM = frozenset({"go", "approve", "abandon", "allow", "trust new host key"}) # Near-misses for abandon: asked about, never taken for it or for a reply. _NEAR_ABANDON = frozenset( @@ -125,6 +128,8 @@ _NEAR_ABANDON = frozenset( ) BUG_GRACE_S = 600.0 GENERAL_IDLE_S = 86400.0 +_SAVED = "The work in progress is pushed; anything else on it will be lost." +BROKEN = "broken" # a VM block's cause: the driver found it broken (D3) PROGRESS_EVERY_S = 60.0 # progress in prose at a readable rate (L5, #42) PROGRESS_CHARS = 400 DETAILS = 10 # tool calls shown by `details` @@ -162,6 +167,11 @@ 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. +If the build VM itself seems broken, fix it the usual ways first: reinstall +the project's dependencies, or a toolchain_step if the toolchain is wrong. If +that doesn't help, don't spend the work diagnosing it: request_reimage, and it +is rebuilt from its template. + Before each group of tool calls, say in a sentence what you're doing and why: the operator sees it as your progress. @@ -1273,6 +1283,16 @@ class LoopAgent: ) self._schedule() return + ready = _VM_READY.fullmatch(command) + if ready is not None: + self._store.transcript(item.id if item else None, "operator").append( + UserMessage(text=text, source=f"transport:{msg.sender}") + ) + self._transport.post( + self._vm_ready(ready.group(1), msg.sender), thread=msg.thread + ) + self._schedule() + return named = _NAMED.fullmatch(command) if named is not None: # L5: a command that names its item applies to it. @@ -1814,9 +1834,21 @@ class LoopAgent: """Ready items in the order they run (L1): the operator's order first, then the oldest.""" items = self._store.work_items("open") # oldest first - ready = [i for i in items if i.position == READY] + blocks = self._store.vm_blocks() + ready = [ + i + for i in items + if i.position == READY and self._host_of(i.scope) not in blocks + ] return sorted(ready, key=lambda i: -loop_of(i).first) + def _host_of(self, project: str | None) -> str | None: + """The build VM a project's work runs on, as blocks name it (D3).""" + found = self._projects.get(project or "") + if found is None or found.workspace is None: + return None + return found.workspace.host.describe() + def status(self) -> str: items = self._store.work_items("open") if not items: @@ -1839,6 +1871,10 @@ class LoopAgent: what += f", going ahead in {left} min unless you reply" elif item.position == BLOCKED: what = f"blocked by {', '.join(loop.blocked_by)}" + elif item.position == READY and item.id not in queue: + block = self._store.vm_blocks().get(self._host_of(item.scope) or "") + why = block.reason if block else "unknown" + what = f"blocked: its build VM needs recovery ({why})" elif item.position == READY: what = f"ready, {queue.index(item.id) + 1} of {len(queue)} in line" name = ( @@ -2425,6 +2461,7 @@ class LoopAgent: tools = [ *([] if hold else [RESTATE]), ASK_HUMAN, + *([REQUEST_REIMAGE] if checked else []), *( _LOOK if not loop.started @@ -2656,6 +2693,8 @@ class LoopAgent: question = str(args.get("question", "")).strip() self._answer_where_asked(item, loop, question) self._wait(item, replace(loop, wait=ANSWER, note=question), None) + elif outcome.kind == "wait" and outcome.tool == "request_reimage": + self._reimage(item, loop, str(args.get("reason", "")).strip()) elif outcome.kind == "wait" and outcome.tool == "wait_for_review": note = str(args.get("note", "")).strip() or f"waiting on PR #{loop.pr}" if loop.pr: @@ -2699,6 +2738,105 @@ class LoopAgent: self._wait(item, replace(loop, wait=ANSWER, note=note), note) self._store.set_active(None, cause="item waiting") + def _reimage(self, item: WorkItem, loop: Loop, reason: str) -> None: + """P5, D3: the driver found its build VM broken past the usual fixes. + The VM is blocked, so nothing runs on it, until the operator says + `vm ready`; the item that found it goes first after that.""" + project = self._at(item) + host = self._host_of(project.name) + if host is None: + note = f"The build VM seems broken: {reason}. Reply once it's fixed." + self._wait(item, replace(loop, wait=ANSWER, note=note), note) + return + self._fact(item, f"build VM reimage requested: {reason}") + # Nothing of the work may live only on a VM about to be wiped: park + # it, as for a switch (pushed to Gitea). The VM may be too broken. + risk = "" + if project.milestones is not None: + try: + project.milestones.park(keep=self._keep(project.name)) + except Exception as exc: # noqa: BLE001 -- said; the reimage loses it + risk = ( + f"I couldn't push the work in progress first ({exc}): it " + "will be lost with everything else on it." + ) + self._fact(item, f"couldn't park the work before a reimage: {exc}") + first = max(loop_of(i).first for i in self._store.work_items("open")) + 1 + save( + self._store, + item.id, + READY, + replace(loop, first=first, question=False, back_to="", back_note=""), + cause="build VM blocked", + ) + self._block_vm(host, BROKEN, reason, item, risk) + + def _block_vm( + self, host: str, cause: str, reason: str, by: WorkItem, risk: str = "" + ) -> None: + """Block a build VM (D3): every project on it waits until it is + recovered. Said once at top level, and in each waiting item's thread. + The recovery is the operator's for now; nisse-ops' later (M7.6).""" + self._store.block_vm(host, cause=cause, reason=reason, work_item=by.id) + names = sorted(p for p in self._projects if self._host_of(p) == host) + blocked = [ + i + for i in self._store.work_items("open") + if i.scope in names and i.scope != GENERAL + ] + for other in blocked: + line = ( + f"Blocked: the build VM ({host}) needs reimaging. " + f"Waiting for `vm ready {other.scope}`." + ) + self._fact(other, line) + self._transport.post(line, thread=loop_of(other).thread) + self._transport.post( + f"The build VM {host} seems broken, found working on " + f"{self._label(by)}: {reason}\n\nThe projects on it are blocked " + f"until it's fixed: {', '.join(names)}. Could you reimage it from " + f"its template? {risk or _SAVED} If its " + "address changes, update `build_host` in my configuration and " + "restart me first.\n\nOnce it's ready, say " + + " or ".join(f"`vm ready {n}`" for n in names) + + "; that also trusts its new host key." + ) + + def _vm_ready(self, name: str | None, sender: str) -> str: + """The operator's `vm ready`: the build VM's block lifts and its new + host key is trusted (D3). A reimaged VM has no marker, so its first + run sets it up again from the repository.""" + blocks = self._store.vm_blocks() + if name is None: + names = sorted(p for p in self._projects if self._host_of(p) in blocks) + if len(names) != 1: + return ( + "No build VM is blocked." + if not names + else "Which one? Say " + + " or ".join(f"`vm ready {n}`" for n in names) + ) + [name] = names + if name not in self._projects: + return f"I don't know a project called {name}." + host = self._host_of(name) + workspace = self._projects[name].workspace + if host is None or workspace is None or host not in blocks: + return f"Nothing is blocking {name}'s build VM." + workspace.host.forget_host_key() + block = self._store.unblock_vm(host, cause=f"vm ready from {sender}") + names = sorted(p for p in self._projects if self._host_of(p) == host) + for other in self._store.work_items("open"): + if other.scope in names: + self._fact( + other, + f"the operator ({sender}) said the build VM ({host}) is " + "ready: its new host key is trusted, and it is set up again " + "if it was reimaged", + ) + assert block is not None # checked above, on the worker + return f"{host} is unblocked: {', '.join(names)} can go on." + # -- milestones (D4) ----------------------------------------------------- def _start(self, item_id: str, args: dict[str, object]) -> tuple[str, bool]: diff --git a/nisse-dev/src/nisse_dev/loop/tools.py b/nisse-dev/src/nisse_dev/loop/tools.py index bcef23a..2936c81 100644 --- a/nisse-dev/src/nisse_dev/loop/tools.py +++ b/nisse-dev/src/nisse_dev/loop/tools.py @@ -78,6 +78,16 @@ TOOLCHAIN_STEP = _tool( reason="What the work needs, and why.", ) +REQUEST_REIMAGE = _tool( + "request_reimage", + "The build VM seems broken beyond the usual fixes (reinstalling " + "dependencies, a toolchain_step): ask for it to be reimaged from its " + "template rather than diagnosing it further. Anything not pushed on it is " + "lost. Ends your turn; the project waits until the operator says the VM " + "is ready, and it is set up again from the repository first.", + reason="What is broken, and what you tried.", +) + # A conversation's tools (L1, L5): it talks, and may propose work. REPLY = _tool( "reply", @@ -122,6 +132,7 @@ WAITING = frozenset( "propose_work", "propose_project", "toolchain_ready", + "request_reimage", } ) diff --git a/nisse-dev/src/nisse_dev/state.py b/nisse-dev/src/nisse_dev/state.py index 6405558..4a68897 100644 --- a/nisse-dev/src/nisse_dev/state.py +++ b/nisse-dev/src/nisse_dev/state.py @@ -29,7 +29,7 @@ from nisse_dev.canonical import ( from nisse_dev.oplog import OpLog from nisse_dev.views import project -CURRENT_SCHEMA_VERSION = 4 +CURRENT_SCHEMA_VERSION = 5 # Stores older than this predate work items (Development Plan M1.1). There was # no production data to carry over, so there is no migration from them: start @@ -38,8 +38,23 @@ OLDEST_SUPPORTED_VERSION = 4 Migration = Callable[[sqlite3.Connection], None] +_VM_BLOCKS = """ + CREATE TABLE vm_blocks ( + host TEXT PRIMARY KEY, + cause TEXT NOT NULL, + reason TEXT NOT NULL, + work_item TEXT, + since TEXT NOT NULL + ) +""" + + +def _add_vm_blocks(conn: sqlite3.Connection) -> None: + conn.execute(_VM_BLOCKS) + + # Maps version N to the migration that takes the schema from N to N+1. -MIGRATIONS: Mapping[int, Migration] = {} +MIGRATIONS: Mapping[int, Migration] = {4: _add_vm_blocks} # Who produced an event (L7): the operator, the orchestrator's own code, or a # model role (L9). planner and executor are the old loop's (until M2.8), kept @@ -77,6 +92,18 @@ class WorkItem: closed_at: str | None +@dataclass(frozen=True) +class VMBlock: + """A build VM nothing may run on until it is recovered (D3): every + project on it waits.""" + + host: str + cause: str # broken (the driver said so); unreachable (#137) + reason: str + work_item: str | None # the item that found it + since: str + + @dataclass(frozen=True) class Intent: id: int @@ -142,6 +169,7 @@ _SCHEMA = [ marked_at TEXT NOT NULL ) """, + _VM_BLOCKS, ] @@ -369,6 +397,34 @@ class StateStore: "work_item.close", cause=cause, work_item=item_id, outcome=outcome ) + # --- build VMs (D3) ---------------------------------------------------- + + def block_vm( + self, host: str, *, cause: str, reason: str, work_item: str | None + ) -> None: + """Block a build VM; a second block on it keeps the first.""" + row = self._conn.execute( + "INSERT INTO vm_blocks (host, cause, reason, work_item, since) " + "VALUES (?, ?, ?, ?, ?) ON CONFLICT(host) DO NOTHING RETURNING host", + (host, cause, reason, work_item, _now()), + ).fetchone() + if row is not None: + self.oplog.emit("vm.block", cause=cause, host=host, work_item=work_item) + + def unblock_vm(self, host: str, *, cause: str) -> VMBlock | None: + """Lift a build VM's block; returns it, or None if there was none.""" + row = self._conn.execute( + "DELETE FROM vm_blocks WHERE host = ? RETURNING *", (host,) + ).fetchone() + if row is None: + return None + self.oplog.emit("vm.unblock", cause=cause, host=host) + return VMBlock(**dict(row)) + + def vm_blocks(self) -> dict[str, VMBlock]: + rows = self._conn.execute("SELECT * FROM vm_blocks").fetchall() + return {row["host"]: VMBlock(**dict(row)) for row in rows} + def carry_idle_messages(self, item_id: str) -> int: """Copy what the operator said while the agent was idle into a work item that has just opened (D1: messages sent before `use` are kept diff --git a/nisse-dev/tests/test_loop_properties.py b/nisse-dev/tests/test_loop_properties.py index b921e22..217fe2f 100644 --- a/nisse-dev/tests/test_loop_properties.py +++ b/nisse-dev/tests/test_loop_properties.py @@ -107,6 +107,9 @@ class PolicyModel: toolchain=[{"name": "python", "version_command": command}], # type: ignore[arg-type] ) ] + if "request_reimage" in offered and self.rng.random() < 0.03: + # M4.3: the build VM seems broken past the usual fixes. + return [call("request_reimage", reason=f"broken {self.n}")] roll = self.rng.random() if roll < 0.08: return [call("ask_human", question=f"question {self.n}?")] @@ -196,7 +199,8 @@ class PolicyModel: class Watched: """The driver's model, watched: a run on a project item (not its - preflight) must find the project's marker on the VM.""" + preflight) must find the project's marker on the VM, and no model runs + for an item whose build VM is blocked.""" def __init__(self, walk: "Walk", inner: Any) -> None: self.walk = walk @@ -211,6 +215,10 @@ class Watched: ) if item is not None and item.scope != "general" and working and not marked: self.walk.unready.append(item.id) + if item is not None and item.scope in self.walk.worlds: + host = self.walk.worlds[item.scope]["host"].describe() + if host in self.walk.world["store"].vm_blocks(): + self.walk.on_blocked.append(item.id) return self.inner.complete(events, tools, model, system) @@ -291,6 +299,7 @@ class Walk: self.markers = Path(self.world["repo"].root).parent / "markers" self.unready: list[str] = [] self.wiped_at: dict[str, int] = {} # project -> last event before a wipe + self.on_blocked: list[str] = [] # items run while their VM was blocked if self.merge == "approved": for w in worlds.values(): self._watch_merges(w["forge"]) @@ -395,6 +404,8 @@ class Walk: "issue": 1, "chat": 1, } + if self.world["store"].vm_blocks(): + weights.update({"vm-ready": 3}) item = self.rng.choice(open_items) if open_items else None loop = loop_of(item) if item else None # The forge an item's project lives on; a new issue picks one. @@ -423,12 +434,20 @@ class Walk: if action == "intake": self.agent.intake() elif action == "wipe": - # The build VM is rebuilt (D3): the clone and its marker gone. - w = self.worlds[project] - shutil.rmtree(w["repo"].root, ignore_errors=True) - events = self.world["store"].all_events() - self.wiped_at[project] = events[-1].id if events else 0 - (self.markers / f"{project}.ready").unlink(missing_ok=True) + self._wipe(project) + elif action == "vm-ready": + # M4.3: the operator recovers a blocked VM, reimaged or not; a + # reimage wipes every project on it. + blocks = self.world["store"].vm_blocks() + on_it = [ + name + for name, w in self.worlds.items() + if w["host"].describe() in blocks + ] + if self.rng.random() < 0.5: + for name in on_it: + self._wipe(name) + self.transport.receive(DON, f"vm ready {self.rng.choice(on_it)}") elif action == "done" and loop: # The operator says a new project is ready; no entry was added. self.transport.receive(DON, "done", thread=loop.thread) @@ -557,6 +576,14 @@ class Walk: self.agent.watch() self.agent._schedule() + def _wipe(self, project: str) -> None: + """The build VM is rebuilt (D3): the clone and its marker gone.""" + w = self.worlds[project] + shutil.rmtree(w["repo"].root, ignore_errors=True) + events = self.world["store"].all_events() + self.wiped_at[project] = events[-1].id if events else 0 + (self.markers / f"{project}.ready").unlink(missing_ok=True) + # -- the properties (specs/loop.qnt) ------------------------------------- def _dangerous_only_when_allowed(self, where: str) -> None: @@ -592,6 +619,10 @@ class Walk: where = f"seed {self.seed}, steps {self.steps}" # M4.1: the driver never works on a project whose VM has no marker. assert not self.unready, f"ready_before_work: {self.unready}: {where}" + # M4.3: nothing runs for a project whose build VM is blocked. + assert not self.on_blocked, ( + f"blocked_vm_runs_nothing: {self.on_blocked}: {where}" + ) # D5: the agent asks Gitea to merge only what an operator approved # as it is (Gitea would refuse the rest; the harness mustn't ask). unapproved = [pr for pr, approved in self.agent_merges if not approved] diff --git a/nisse-dev/tests/test_loop_toolchain.py b/nisse-dev/tests/test_loop_toolchain.py index f0d2f1d..d20b907 100644 --- a/nisse-dev/tests/test_loop_toolchain.py +++ b/nisse-dev/tests/test_loop_toolchain.py @@ -3,7 +3,9 @@ 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.""" +and set up again, and a changed host key waits for the operator. A VM the +driver finds broken blocks its projects until the operator says it's ready +(M4.3).""" import io import shutil @@ -27,6 +29,7 @@ from test_loop_milestones import world as world # noqa: PLC0414 -- the fixture from nisse_dev.cancel import Cancel from nisse_dev.canonical import Fact, ToolResult from nisse_dev.execution import HostKeyChanged +from nisse_dev.gitea import Issue from nisse_dev.loop import preflight from nisse_dev.loop.agent import LoopAgent, Project from nisse_dev.loop.driver import Driver @@ -338,3 +341,130 @@ def test_before_the_go_ahead_the_driver_only_looks( 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 + + +# --- M4.3: a broken VM blocks its projects until it's ready --------------------------- + + +def reimage(reason: str = "pip crashes in every venv, even a new one") -> list[Any]: + return [call("request_reimage", reason=reason)] + + +def by_issue(world: dict[str, Any], number: int) -> Any: + [found] = [i for i in world["store"].work_items() if loop_of(i).issue == number] + return found + + +def facts_of(world: dict[str, Any], item: Any) -> list[str]: + events = world["store"].transcript(item.id, "orchestrator").read() + return [e.event.text for e in events if isinstance(e.event, Fact)] + + +def test_a_broken_vm_blocks_its_project_until_the_operator_says_ready( + world: dict[str, Any], project: str, monkeypatch: pytest.MonkeyPatch +) -> None: + world["forge"].issues.append(Issue(15, "Add count_lines", "count lines", "don", ())) + forgotten: list[str] = [] + monkeypatch.setattr( + type(world["host"]), "forget_host_key", lambda self: forgotten.append("yes") + ) + # Asked for before the go-ahead: it's a request, not an action. + model = ScriptedModel(ready(), reimage(), ready(), RESTATE, RESTATE) + agent, transport = agent_for(world, project, model) + agent.intake() + first, second = by_issue(world, 14), by_issue(world, 15) + assert "build VM reimage requested: pip crashes in every venv, even a new one" in ( + facts_of(world, first) + ) + [asked] = [ + m for m in transport.outbound if "reimage it from its template" in m.text + ] + assert asked.thread is None # top level: it can cover several projects + assert "pip crashes in every venv" in asked.text + assert f"`vm ready {project}`" in asked.text + assert any(f.startswith("Blocked: the build VM") for f in facts_of(world, second)) + # Neither runs while the VM is blocked, even after a restart. + again, _ = agent_for(world, project, ScriptedModel()) + assert again._queue() == [] + assert "its build VM needs recovery" in again.status() + # The operator reimages it: the clone and the marker are gone. + shutil.rmtree(world["repo"].root) + world["host"].execute(f'rm -f "{preflight.marker_path(project, markers(world))}"') + transport.receive(DON, f"vm ready {project}") + assert forgotten == ["yes"] # its new host key is trusted + assert world["store"].vm_blocks() == {} + assert any("cloned the workspace again" in f for f in facts_of(world, first)) + # The item that found it went first, and set the VM up again. + assert loop_of(by_issue(world, 14)).wait == GO_AHEAD + assert loop_of(by_issue(world, 15)).wait == GO_AHEAD + setups = [ + f + for item in (first, second) + for f in facts_of(world, item) + if "setting the build VM up from the repository" in f + ] + assert len(setups) == 2 # the first, and after the reimage + assert setups == [f for f in facts_of(world, first) if "setting the build VM" in f] + + +def test_a_vm_said_ready_without_a_reimage_goes_straight_on( + world: dict[str, Any], project: str +) -> None: + model = ScriptedModel( + ready(), + RESTATE, + [call("start_milestone", name="m"), *reimage("the build hangs")], + [*WORK[1:], *FINISH], + ) + agent, transport = agent_for(world, project, model) + agent.intake() + say(transport, world, "go") + assert world["store"].vm_blocks() + transport.receive(DON, "vm ready") # the only blocked VM: no name needed + # Not reimaged: the marker is still there, so no new setup step. + assert ( + sum("setting the build VM up from the repository" in f for f in facts(world)) + == 1 + ) + assert world["forge"].prs # the work went on to a PR + + +def test_vm_ready_with_nothing_blocked_says_so( + world: dict[str, Any], project: str +) -> None: + _, transport = agent_for(world, project, ScriptedModel()) + transport.receive(DON, "vm ready") + transport.receive(DON, f"vm ready {project}") + transport.receive(DON, "vm ready nowhere") + assert said(transport) == [ + "No build VM is blocked.", + f"Nothing is blocking {project}'s build VM.", + "I don't know a project called nowhere.", + ] + + +def test_work_in_progress_is_pushed_before_a_reimage( + world: dict[str, Any], project: str +) -> None: + model = ScriptedModel( + ready(), + RESTATE, + [ + call("start_milestone", name="m"), + call("write_file", path="w.py", content="x = 1\n"), + *reimage(), + ], + ready(), # set up again after the reimage + [call("finish_milestone", summary="w.py, tested", final="yes")], + ) + agent, transport = agent_for(world, project, model) + agent.intake() + say(transport, world, "go") + [asked] = [t for t in said(transport) if "reimage it from its template" in t] + assert "The work in progress is pushed" in asked + shutil.rmtree(world["repo"].root) + world["host"].execute(f'rm -f "{preflight.marker_path(project, markers(world))}"') + transport.receive(DON, f"vm ready {project}") + forge = world["forge"] + [pr] = [n for n, p in forge.prs.items() if not p["head"].startswith("nisse-check")] + assert "w.py" in forge.files(pr) # it survived the reimage diff --git a/nisse-dev/tests/test_state.py b/nisse-dev/tests/test_state.py index b392135..25861b3 100644 --- a/nisse-dev/tests/test_state.py +++ b/nisse-dev/tests/test_state.py @@ -121,3 +121,28 @@ def test_newer_schema_is_refused(db: Path) -> None: pass with pytest.raises(StateSchemaError, match="newer"): StateStore(db, schema_version=1) + + +def test_a_vm_block_persists_across_reopen(db: Path) -> None: + with StateStore(db) as store: + store.block_vm("root@vm1", cause="broken", reason="pip crashes", work_item="x") + store.block_vm("root@vm1", cause="broken", reason="later", work_item="y") + with StateStore(db) as store: + [block] = store.vm_blocks().values() + assert (block.host, block.reason, block.work_item) == ( + "root@vm1", + "pip crashes", # the first block is kept + "x", + ) + assert store.unblock_vm("root@vm1", cause="test") == block + assert store.unblock_vm("root@vm1", cause="test") is None + assert store.vm_blocks() == {} + + +def test_a_version_4_store_gains_vm_blocks(db: Path) -> None: + with StateStore(db, schema_version=4) as store: + store._conn.execute("DROP TABLE vm_blocks") # as a real version 4 store + with StateStore(db) as store: + assert store.schema_version == 5 + store.block_vm("root@vm1", cause="broken", reason="r", work_item=None) + assert list(store.vm_blocks()) == ["root@vm1"] diff --git a/specs/loop.qnt b/specs/loop.qnt index 4370a52..825869d 100644 --- a/specs/loop.qnt +++ b/specs/loop.qnt @@ -61,6 +61,8 @@ module loop { opened: int -> int, // PR key -> PRs ever opened for it // -- durable: Matrix holds messages until the agent handles them -- inbox: List[Msg], + // -- durable: the state store's VM blocks (D3) -- + vmBlocked: Set[str], // projects whose build VM waits for recovery // -- volatile -- work: str, // none / publish pubStage: int, @@ -70,6 +72,7 @@ module loop { earlyMs: bool, // a milestone started before the previous merged lostFix: bool, // a review fix pushed to a PR no longer open lostWork: bool, // a parked item's unpushed work was set aside or wiped + ranBlocked: bool, // an item was taken up on a blocked build VM } var s: Sys @@ -98,9 +101,10 @@ module loop { items: Map(), nextItem: 1, active: 0, intents: Set(), issues: ISSUES.mapBy(_ => { open: false, assigned: false, linked: 0 }), depOpen: DEP_ISSUES.mapBy(_ => true), - prs: Map(), opened: Map(), inbox: List(), + prs: Map(), opened: Map(), inbox: List(), vmBlocked: Set(), work: "none", pubStage: 0, pending: 0, badGo: false, earlyMs: false, lostFix: false, lostWork: false, + ranBlocked: false, } // ---- the operator ---------------------------------------------------------- @@ -236,10 +240,12 @@ module loop { // sets aside whatever is left there. action activate(i: int): bool = all { s.active == 0, s.work == "none", live(s).contains(i), itm(s, i).status == "ready", + not(s.vmBlocked.contains(itm(s, i).project)), // D3: nothing runs on a blocked VM s' = { val leftovers = s.items.keys().exists(j => j != i and itm(s, j).project == itm(s, i).project and itm(s, j).unpushed) - { ...s, active: i, lostWork: s.lostWork or leftovers } + { ...s, active: i, lostWork: s.lostWork or leftovers, + ranBlocked: s.ranBlocked or s.vmBlocked.contains(itm(s, i).project) } }, } @@ -392,6 +398,30 @@ module loop { j != s.active and itm(s, j).project == p and itm(s, j).unpushed) }, } + // The driver finds its build VM broken (request_reimage, D3): its work is + // parked (pushed), the VM blocked and the run ends; the item stays ready, + // to go on once it's fixed. (A VM too broken to push loses the work; the + // operator is told before reimaging it, accepted as for any wipe.) + action requestReimage: bool = { + val it = itm(s, s.active) + all { + s.active != 0, s.work == "none", s.intents.size() == 0, it.status == "ready", + s' = { ...upd(s, s.active, { ...it, unpushed: false }), + vmBlocked: s.vmBlocked.union(Set(it.project)), active: 0 }, + } + } + + // The operator's `vm ready`: the block lifts. A reimage loses what wasn't + // pushed on the VM; the operator was told so, and agreed by doing it. + action vmReady(p: str, reimaged: bool): bool = all { + s.vmBlocked.contains(p), + s' = { ...s, + vmBlocked: s.vmBlocked.exclude(Set(p)), + items: if (reimaged) s.items.keys().mapBy(j => + if (itm(s, j).project == p) { ...itm(s, j), unpushed: false } else itm(s, j)) + else s.items }, + } + // ---- crash and boot (L8) -------------------------------------------------- // Volatile state is lost; boot reconciles each pending intent against @@ -427,7 +457,7 @@ module loop { intake(anyIssue, p, b), observe(i), handleMsg, park, activate(i), restate(b), graceOver(i), createRecord, linkRecord, startMilestone, turn(outcome), finishMilestone, publishStep(b), answerReview, - finish, wipe(p), crash, + finish, wipe(p), crash, requestReimage, vmReady(p, b), } } @@ -473,9 +503,13 @@ module loop { s.intents.forall(k => k / 100 != i), }) + // Nothing is taken up on a build VM that is blocked (D3). + val blocked_vm_runs_nothing = not(s.ranBlocked) + val all_properties = and { one_item_per_issue, waits_are_explainable, go_binds, milestone_after_merge, parked_work_safe, one_pr_per_milestone, review_fixes_land, closed_items_leave_nothing, + blocked_vm_runs_nothing, } // ---- reachability witnesses (each should be violated) -------------------------- @@ -485,5 +519,6 @@ module loop { val w_blocked = not(s.items.keys().exists(i => itm(s, i).status == "blocked")) val w_review = not(s.items.keys().exists(i => itm(s, i).review)) val w_record = not(s.items.keys().exists(i => itm(s, i).src == "conv" and itm(s, i).issue != 0)) + val w_vm_blocked = s.vmBlocked.size() == 0 val w_finished = not(s.items.keys().exists(i => itm(s, i).status == "closed" and itm(s, i).ms > 0)) } diff --git a/specs/loop_scenarios.qnt b/specs/loop_scenarios.qnt index 1d793d7..d031526 100644 --- a/specs/loop_scenarios.qnt +++ b/specs/loop_scenarios.qnt @@ -74,4 +74,15 @@ module loop_scenarios { .expect(s.issues.get(99).linked == 1) .then(linkRecord) .expect(one_item_per_issue and itm(s, 1).issue == 99) + + // M4.3: a build VM the driver finds broken blocks its project; nothing is + // taken up on it until the operator says `vm ready`, then work goes on. + run aBrokenVmBlocksUntilReadyTest = + init + .then(assignIssue(1)).then(intake(1, "A", false)) + .then(activate(1)).then(requestReimage) + .expect(s.vmBlocked.contains("A") and s.active == 0) + .then(vmReady("A", true)) + .then(activate(1)) + .expect(blocked_vm_runs_nothing and s.active == 1) }